Agree
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Lint Skipped - Unit
Unit Tests Skipped - Build Status
Buildable 6464 Build 10704: Vulcan Build Jenkins
Event Timeline
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/797/
binaries/data/mods/public/gui/pregame/mainmenu.js | ||
---|---|---|
93 | (Anyone else feel this is a pretty ugly way to show the splashscreen?) |
binaries/data/mods/public/gui/pregame/mainmenu.js | ||
---|---|---|
93 | Feel free to submit a patch :) |
Stan can you describe how this number is used? It is not clear to me from a quick read and quick test (and why 1 is better than 1.7777?)
binaries/data/mods/public/gui/pregame/mainmenu.js | ||
---|---|---|
93 | /me raises hand |
Basically this number is multiplied to whatever the height is. So in case it's 1080 you get 1920 if it's 1050 you get 1866 instead of 1680 (Because it's supposed to be a 16:10 ratio) if you give it 768 (for 1024x768 which is the minimun resolution we support) you get 1366. That is not a 4:3 ratio.
I understand that it's a multiplication, but what is the purpose of the number?
Edit: rP10042 philip var w = h*16/9; // width of background image
It's only used to compute the offset of the background. it's passed as an argument to the background to offset it.
Each background defines for each of it's sprites this:
{ "offset": (time, width) => 0.02 * width * Math.cos(0.05 * time), "sprite": "background-kush1-1", "tiling": true, },
Not to mention the computation is really innefficient because it computes it one time per layer.
Just mere guessing, but doesn't that hardcode the ratio of the image rather than the ratio of the screen and tries to make sure that the image is moved into all directions with equal speed?
The problem with the current code is that at no time we recover the real size of the backgrounds.
Depending on the window size the tiling backgrounds are more or less fully displayed.
Test is better than words.
Sets a window size of 1280x1024 by example and uses the Kushite background.
At the end of the scroll to the right, the right side of the city is still not displayed.
Function was moved to BackgroundHandler.js.
For me h * 16/9 = 1818.6666666666667 and screen.right - screen.left = 1916.
One can set different w values to see how it impacts the animation.
For example w=2 = no background scrolling, everything looks fine.
and w=2000000 super fast scrolling, the background is repeating, otherwise fine.
Fine as in the images are displayed in proportion.
If there are some elements of the picture that should be displayed but aren't, like Polakrity said, then this may be accounted for in the background file as well.
I suppose it would be good to test tiling backgrounds, somehow, or become sure that it works for them as well in theory.
binaries/data/mods/public/gui/pregame/mainmenu.js | ||
---|---|---|
93 |
In D2390 you wrote:
This is still not a good idea to hardcode that (though it's not related to that patch directly)
Because this should be computed and/or cached and updated on window resize. Because the minimum resolution we claim to support is 1024/768 and that's a 4:3 ratio
But I don't see why "this should be computed and/or cached and updated on window resize" nor why this has anything to do with the minimum resolution or the screen ratio at all.
If I use a 4:3 monitor should the icons on the desktop use 4:3 ratio too, and for 16:9 they should use 16:9 ratio?
You can put literally every value in there and it will work correctly without showing any black bars, aspect ratio of 0.0001 or 1000 or without changing the aspect ratio of the image.
What it does change the is the width of the backdrop scrolled, and it repeats the image with tile=true and it does not repeat it with tile=false (in which case the author must ensure that the formula works in such a way to always render something at the background).