Page MenuHomeWildfire Games

Smooth new Kushite background
ClosedPublic

Authored by Polakrity on Jan 24 2019, 5:40 AM.

Details

Reviewers
Stan
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22259: Fix the kush background stuttering.
Summary

In the same spirit as #D982, fix something not really notifiable.

There are small slutterings when the Kushite background is scrolling.
Adding the round_coordinates attribute for all sprites fix it.

Better display of balcony with a hack.
(The balcony isn't fully displayed following the resolution of the user's screen maybe we can scale it a bit.)

Test Plan

Focus your attention on the elements of background like balcony, clouds.. and notice the slutterings.
Apply the patch and see the little difference.

*You can delete the others backgrounds in public\gui\pregame\backgrounds to avoid displaying them.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 6832
Build 11231: Vulcan BuildJenkins

Event Timeline

Polakrity created this revision.Jan 24 2019, 5:40 AM
Vulcan added a subscriber: Vulcan.Jan 24 2019, 5:43 AM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/pregame/backgrounds/kush.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/pregame/backgrounds/kush.js
|  16|  16| 			"tiling": true,
|  17|  17| 		},
|  18|  18| 		{
|  19|    |-			"offset": (time, width) => -55,	
|    |  19|+			"offset": (time, width) => -55,
|  20|  20| 			"sprite": "background-kush1-4",
|  21|  21| 			"tiling": false,
|  22|  22| 		},
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/991/

Polakrity edited the summary of this revision. (Show Details)Jan 24 2019, 5:47 AM
Stan added a reviewer: Stan.Jan 24 2019, 8:28 AM
Stan added a subscriber: Stan.

The reason I offsetted it a little is for 16/10 resolution and 21/9 to reduce the damage.

Did you check those resolutions as well as 1024x768 ?
Someone reported the clouds moved too fast can you check ?
Thanks for the patch !

binaries/data/mods/public/gui/pregame/backgrounds/kush.xml
30

This seemed to have no effect whatsoever' are you sure it's useful ?

Polakrity added a comment.EditedJan 24 2019, 12:11 PM

I didn't really notice the clouds move too fast but maybe its just me.

Polakrity marked an inline comment as done.Jan 24 2019, 12:12 PM
Polakrity added inline comments.
binaries/data/mods/public/gui/pregame/backgrounds/kush.xml
30

I added this without really take account of what is it.
I think this wrapping effect was useful for avoid the occurence of black border.
But since rev 16747 its use has become useless.

Stan added inline comments.Jan 24 2019, 12:50 PM
binaries/data/mods/public/gui/pregame/backgrounds/kush.xml
30

Do you know why, If I could clamp the sprite to the edge that would be way better than manually offsetting it.

Polakrity updated this revision to Diff 7383.Jan 24 2019, 2:08 PM

Update patch:
Maybe a better balcony scale.
And remove the effect.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/994/

Stan added inline comments.Jan 24 2019, 11:57 PM
binaries/data/mods/public/gui/pregame/backgrounds/kush.js
19

If you reduce the height of the window it will break IIRC hence why I left the guard a bit on the edge. Clamping would be nice ...

Polakrity updated this revision to Diff 7465.Feb 7 2019, 3:24 PM
Polakrity marked an inline comment as done.
Vulcan added a comment.Feb 7 2019, 3:25 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1050/

nani added a subscriber: nani.Feb 7 2019, 3:36 PM

I can also feel the clouds move too fast relative to the city+river+desert+near mountain (looks like if the clouds are very close).

Also the parallax effect for the far distant mountains+sky doesn't feel coordinated with city+river+desert+near mountain one

Stan added a comment.Feb 7 2019, 3:51 PM

@nani Feel free to suggest values.

@Polakrity if I understand correctly the piece of code you added in mainmenu.js allows the clamping ? :) If so, maybe it would be be nice to check for the sprite's attribute instead of the offset ?

What do you mean by the sprite's attribute?

And I just visualized the kush1_1.png image.
Is it desired to not display the desert in the menu?

nani added a comment.EditedFeb 8 2019, 6:27 PM

How about this?

		{
			"offset": (time, width) => 0.07 * width * Math.cos(0.1 * time),
			"sprite": "background-kush1-1",
			"tiling": true,
		},
		{
			"offset": (time, width) => 0.05 * width * Math.cos(0.1 * time),
			"sprite": "background-kush1-2",
			"tiling": true,
		},
		{
			"offset": (time, width) => 0.04 * width * Math.cos(0.1 * time) + 0.01 * width * Math.cos(0.04 * time),
			"sprite": "background-kush1-3",
			"tiling": true,
		},
		{
			"offset": (time, width) => -40,
			"sprite": "background-kush1-4",
			"tiling": false,
		}
Stan added a reviewer: Restricted Owners Package.Feb 24 2019, 5:02 PM

I can't review code change alone, adding the GUI guys to the queue.

Before the patch the balkon position was wrong.


Now after the patch the balkon is on the right position but stretched.

Stan added a comment.Apr 23 2019, 2:56 PM

@OptimusShepard Is that due to the js changes ?

Polakrity updated this revision to Diff 7825.EditedApr 24 2019, 3:44 AM
Polakrity edited the summary of this revision. (Show Details)

Use a hack instead of JS modifications.
The JS changes were almost the same as the code for tilling.

Add also the changes proposed by @nani

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1261/display/redirect

Polakrity added a comment.EditedApr 24 2019, 4:05 AM

It should be determined if we want certain elements of the background can't be stretched.
But it can also cause this sprite could be too small compared to other elements for wide screens.

It could be added also the possibility to specify the initial position of a background.
I think that's what you meant by clamp @Stan .

In all cases, this diff is intended to improve the current display of the background.
And for the moment, I prefer to not change the current state of the code.

Stan added a comment.EditedApr 24 2019, 10:17 AM

It could be added also the possibility to specify the initial position of a background.
I think that's what you meant by clamp @Stan .

Actually I made a typo, I meant "clip to bounds" I just want the balcony to stick to the right side with no stretching no matter the stretch of the window size.

binaries/data/mods/public/gui/pregame/backgrounds/kush.xml
23

So I assume by default it's true ?

Stan accepted this revision.May 9 2019, 5:59 PM

This fixes the stuttering issue, there is a reasonable amount of stretching on a 21/9 resolution, so I will commit it.

This revision is now accepted and ready to land.May 9 2019, 5:59 PM
This revision was automatically updated to reflect the committed changes.
Owners added a subscriber: Restricted Owners Package.May 9 2019, 7:46 PM