Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change how is Retina handled on iOS #3709

Merged
merged 3 commits into from Oct 13, 2020
Merged

Conversation

Darkyenus
Copy link
Contributor

This is a breaking change, but makes the API consistent with the core API.

It gets rid of the configuration.displayScaleIf fields in favor of correct getWidth/Height and getBackBufferWidth/Height implementations. This makes development of cross platform hi-dpi applications much easier.

Input and getW/H previously returned pixels, now it returns points.

@davebaol davebaol added the ios label Jan 12, 2016
@badlogic
Copy link
Member

I agree we should unify this now that we have the HDPI support in core. However, using a float to calculate the backbuffer width/height is less than ideal, especially since functions like glViewport/glScissor rely on integer pixel sizes. We'll need to find a way to get the backbuffer size properly in pixels, otherwise we'll run into off-by-one errors.

@badlogic
Copy link
Member

Please also add an explicite note to CHANGES as this is a big API breaking change!

@Darkyenus
Copy link
Contributor Author

The reason for floats is that iOS reports everything in doubles. If I am not mistaken, there is currently no legal way for it to report fraction values, so the code should always result in correct dimensions. However if it does indeed return fractions sometimes in the future, it may not break (that much) if we use doubles/floats as well. But I can amend it so it rounds first and then works in integers.

I don't usually put CHANGES in PR because CHANGES change often and then it results in merge conflicts, but I can add it.

@Darkyenus
Copy link
Contributor Author

Fixed merge conflicts.
I am still not sure what to do about the integers/floats thing. I'm leaning towards "cast doubles to ints and store it all in IntRectangle". Thoughts?

@Darkyenus Darkyenus mentioned this pull request Feb 1, 2016
@Darkyenus
Copy link
Contributor Author

I have made a few changes:

I will add CHANGES when this gets approved, so it doesn't cause merge conflict.

@Darkyenus
Copy link
Contributor Author

Minor change: back buffer dimensions are now be taken from GLKView directly (when available) so they will always be pixel perfect.

Had to revert that because it didn't play well with offsetting for status bar.

@Tom-Ski
Copy link
Member

Tom-Ski commented Mar 10, 2016

Thanks for the PR @Darkyenus, I've pushed changes that break your PR's ability to be merged. If you could fix those by just pushing your changes to retina and squash to a commits I can merge.

This is a breaking change, but makes the API consistent with core.
Fixes libgdx#3789 (resize() being triggered with same dimensions)
Fixes touch mapping on older iOS versions when screen is rotated
@Darkyenus
Copy link
Contributor Author

Squashed

@cypherdare
Copy link
Member

@Darkyenus
FYI

@Darkyenus
Copy link
Contributor Author

@cypherdare I know, but this probably won't get merged anyway, judging by recent events and the fact that this has been opened for quite some time, without any activity.

@Istenes
Copy link

Istenes commented Jun 9, 2016

Hey @Darkyenus, how would you recommend us build libgdx with your fix? Checkout your commit and build libgdx using ant or try to monkey patch it in somehow?

@Darkyenus
Copy link
Contributor Author

@Istenes You have probably figured it already, but if not, here is how I would do it:

  • git pull libgdx
  • git fetch this PR's branch to a different branch
  • cherry pick this PR onto the libGDX master
  • build libGDX normally

@obigu
Copy link
Contributor

obigu commented Jun 27, 2016

Thanks for the fix @Darkyenus, have received reports from several users using iOS 7 saying the app was unusable and this fixes it. Given that RoboVM is still the best choice for iOS backend I think this PR is important and should be merged.

@florianf
Copy link
Contributor

@badlogic, @Tom-Ski is there anything that prevents this from beging merged, since the issue seems to popup repeatedly?

@obigu obigu self-assigned this Oct 5, 2020
# Conflicts:
#	backends/gdx-backend-robovm/src/com/badlogic/gdx/backends/iosrobovm/IOSApplication.java
#	backends/gdx-backend-robovm/src/com/badlogic/gdx/backends/iosrobovm/IOSGraphics.java
#	backends/gdx-backend-robovm/src/com/badlogic/gdx/backends/iosrobovm/IOSInput.java
@obigu
Copy link
Contributor

obigu commented Oct 5, 2020

I've pushed changes to merge master to resolve conflicts.

I think @Darkyenus did a good job with this PR and even if it's old it's still relevant. It improves consitency between backends, makes hdpi on the iOS backend closer to Apple's idea, it makes it more future proof and flexible and the code (as well as the ios configurations is cleaner.

The drawback is that it does break backwards compatibility.

Points and pixels are now different things so all apps that currently use IosGraphics.getWidth() and IosGraphics.getHeight() to get real screen pixels will need to replace them with getBackBufferWidth() and getBackBufferHeight() instead. Also if anybody used the IOSApplicationConfiguration options displayScaleXXXXScreenIfYYYRetina they will need to do it programatically now (I wonder if anybody uses them though).

I've tested it on my projects and everything seems to work but considering the nature of the changes, more testing would be great.

@MrStahlfelge
Copy link
Member

MrStahlfelge commented Oct 6, 2020

I've tested the latest code with my project and it works so far.
I also think the code is more clean and future proof than the old one, so I'd suggest to merge this in after next release.

When we are going to change getWidth()/getHeight() anyway, maybe we should also merge the fix from #5814

@MrStahlfelge MrStahlfelge added the graphics Concerning Gdx.graphics label Oct 6, 2020
@obigu
Copy link
Contributor

obigu commented Oct 8, 2020

I've been working on Split screen and have a PR ready but I'd rather wait until this is merged to avoid conflicts. If we merge this one now before 1.10.0 we can get both in while having them on SNAPSHOT for long enough for more people to test them.

@crykn
Copy link
Member

crykn commented Oct 8, 2020

The problem with merging before 1.10 is that afaik 1.10 is not supposed to contain any breaking changes (see #6154 which is on hold for the same reason).

@obigu
Copy link
Contributor

obigu commented Oct 8, 2020

Sorry if I've created confusion, I was assuming next release would be 1.10.0 (and that we would skip 1.9.12) considering the roadmap for end of October release. That's why I though it made sense to merge it now but If 1.9.12 is going to happen then I agree we should wait to 1.10.0.

@crykn crykn added this to the Next Major Release milestone Oct 8, 2020
@MrStahlfelge MrStahlfelge merged commit 858dc54 into libgdx:master Oct 13, 2020
@Trurl101
Copy link
Contributor

Trurl101 commented Oct 27, 2020

I think this is a bad change and I am not exactly sure it is working as intended.

  • If I start a ios app with 1.9.12-SNAPSHOT it does not start in native resolution anymore but it gets scaled. With 1.9.11. the app started in native resolution, but there seems no way to accomplish native resolution now on IOS with libgdx.

  • All previous math that used the screen size as base now have to deal with a much smaller size. They calculated with the real screensize before and it worked perfectly on all devices (android ios, desktop). Now ios devices return a much smaller screen size. For example before 1242 width now IOS shows only 414 width. It gets scaled up but such huge differences between real and virtual size in combination with the iOS upscale could result in presentation errors, so that some math gives wrong pixel positions.

For example I calculated the (freetype) font size by screen width, worked across all devices (android, ios, desktop) now I must calculate it with a special function for iOS. Other calculations that worked perfectly on all dives and resolutions have now some pixel shifts on iOS too.

In the end iOS devices must now handled different from all other devices, before handling across all devices in libgdx was consistent.

I don't care that this change aligns better with "Apples ideas" in regards to handling different screen resolutions. Apple ideas for resolution handling are just bad. Libgdx should not implement such vendor specialized design ideas but stay consistent across all devices.

@MrStahlfelge
Copy link
Member

MrStahlfelge commented Oct 27, 2020

In the end iOS devices must now handled different from all other devices

That is not correct. For example, you should just calculate your freetype font size based on backbuffer size on all platforms.

stay consistent across all devices

Exactly this was the attempt here. Libgdx core supports the difference between logical and physical screen size for a long time now. Unfortunately, only Lwjgl3 backend made use of this. Now, another step was done to support it on iOS. GWT will follow for sure.

@Trurl101
Copy link
Contributor

Trurl101 commented Oct 27, 2020

In the end iOS devices must now handled different from all other devices

That is not correct. For example, you should just calculate your freetype font size based on backbuffer size on all platforms.

That is of course true but. And I already implemented this. But if I calculate screen positions for some objects with backbuffer size (or the screen width) I could used this position regardless of the device. Now I must scale it down for iOS, before 1.9.12. I could just use the same calculated value for all devices.

stay consistent across all devices

Exactly this was the attempt here. Libgdx core supports the difference between logical and physical screen size for a long time now. Unfortunately, only Lwjgl3 backend made use of this. Now, another step was done to support it on iOS. GWT will follow for sure.

I can see why some developer might think this is a change for better if they never calculated their screen designs to fit the real device screen perfectly but were satisfied with approximate values (that look way worse in my opinion).

The libgdx logical and physical screen size and the iOS scaling are some different things. For example I never used libgdx scaling anyway (that looks often weird and cheap). My games just use the real screen sizes on all devices and it worked perfectly.

Now with 1.9.12. virtual screen sizes are enforced with libgdx on iOS and it will makes headaches for all developer who use native screen sizes in their apps. Because the upscaling and calculation methods must now have special calculations for iOS otherwise it will result in pixel shifts.

There should at least be an option to use the physical resolution on iOS as it was before. Why take away such important options from developers?

@obigu
Copy link
Contributor

obigu commented Oct 27, 2020

For example I calculated the (freetype) font size by screen width, worked across all devices (android, ios, desktop) now I must calculate it with a special function for iOS. Other calculations that worked perfectly on all dives and resolutions have now some pixel shifts on iOS too.

You mention 2 different issues here. The first one seems not to be a problem, as @MrStahlfelge has said, using backbuffer size to determine the real size of the screen to load/generate the correct assets is the way to go.

Regarding the second issue. If I've understood correctly, your game has a mapping of 1 world unit -> 1 pixel which means you use (or do the same as) a ScreenViewport. Using that kind of viewport will cause working with different screen sizes quite complicated but that's your choice. So, to have world units to be real pixels instead of logical pixels (to have the same behaviour as you had before) you could set setUnitsPerPixel() to the pixel density of the device getBackBufferWidth() / getWidth() on your ScreenViewport.

@Trurl101
Copy link
Contributor

Regarding the second issue. If I've understood correctly, your game has a mapping of 1 world unit -> 1 pixel which means you use (or do the same as) a ScreenViewport. Using that kind of viewport will cause working with different screen sizes quite complicated but that's your choice.

Its much easier and give games a more high-quality look, anyway:

So, to have world units to be real pixels instead of logical pixels (to have the same behaviour as you had before) you could set setUnitsPerPixel() to the pixel density of the device Gdx.app.graphics.getDensity() on your ScreenViewport.

Before 1.9.12 I could just use native resolution for all devices and could position objects accordingly. Now I have to include extra calculations for iOS to the virtual resolutions.

This change is fine for people who don't care about native resolutions. An option to just use native resolution on iOS as before would be appreciated.

@obigu
Copy link
Contributor

obigu commented Oct 31, 2020

Its much easier and give games a more high-quality look, anyway:

It is definitely not easier to have a 1 pixel - 1 world unit mapping than to use predictable bounded world sizes once you understand how Viewports work. And the quality look is (or should) be independent from the viewport you use.

Before 1.9.12 I could just use native resolution for all devices and could position objects accordingly. Now I have to include extra calculations for iOS to the virtual resolutions.

This change is fine for people who don't care about native resolutions. An option to just use native resolution on iOS as before would be appreciated.

Caring about native resolutions to make sure your game looks perfect on all devices (which is a good thing) is different from having to care about them every time you position an object on your game world. You have not explained the exact issue you are facing regarding "pixel shifts" or if the workaround I provided (that requires changing 1 single line) works for you.

My advice is we move this conversation to the #ios channel on Discord where, with more information, we can investigate further whether there's a use case in which this change could be limiting or maybe you discover a better way to deal with different device resolutions.

@davidgiga1993
Copy link

davidgiga1993 commented Nov 4, 2020

I somewhat agree with @Trurl101. My app (not a game) also uses a 1:1 mapping. The UI elements are sized using the current display density so they are physically the same size on all devices, platforms and OS scaling.
After this change only ios is broken. Retina screens on windows or osx (lwjgl3) are still working as expected.
This would imply that the ios implementation of the screen size reporting is different than on pc.

@Darkyenus
Copy link
Contributor Author

After this change the behavior on iOS and on lwjgl3 is the same. If you are experiencing different behavior, you are introducing it yourself or you aren't using proper APIs for the task (getWidth/getBackBufferWidth).

If you insist on using the old behavior, just replace all calls to getWidth with calls to getBackBufferWidth (and Height, of course), then you can handle the scaling yourself. However if you do and don't match what getWidth gives you, your app won't feel native as it will have different scaling from every other application on the OS.

As obigu mentioned, Discord is the place for these discussions, unless you think that there is a real bug.

@Trurl101
Copy link
Contributor

Trurl101 commented Nov 4, 2020

If you insist on using the old behavior, just replace all calls to getWidth with calls to getBackBufferWidth (and Height, of course), then you can handle the scaling yourself. However

That doesn't help. If I have have a backbufferwidth of 800px and libgdx width of 444 I must calculate the new position. On all other platforms I can just use the "real" native position.

As obigu mentioned, Discord is the place for these discussions, unless you think that there is a real bug.

Is the future of libgdx to not support development for native resolutions anymore? If the libgdy maintainer think only virtual resolutions are the way to go it will be a big change. I think libgdx should be produce the same results on all platforms if you use the same code, but with this change in 1.9.12. iOS development is practically broken and force developers to change the code often in a big way.

There should be implemented some kind of config setting developers could use to turn on "old" but better/more consistent behaviour for iOS development.

@MrStahlfelge
Copy link
Member

If the libgdy maintainer think only virtual resolutions are the way to go it will be a big change

The maintainers are part of the community, and the community is what drives the project. The PR was open for 4 years with no one from said community chiming in and telling it is a bad idea or that it should be configurable.

Everyone throwing in the effort to test this had zero problems. If you want to participate in decisions what is merged, take part of the process beforehand. Most PRs are left open before merging after approval for a time for a reason - this reason is to give others the chance to give input.

@obigu
Copy link
Contributor

obigu commented Nov 4, 2020

@davidgiga1993 reported an issue on getDensity() that could potentially break stuff for dpi based layouts and will be fixed here #6263. I'm confident that change will solve his issues without having to modify his code.

@Trurl101 I'm not sure what your issue is but, unless you report what you consider is a bug and be a bit specific we won't be able to help you here. Again, we will happily help you on Discord #ios channel.

@penkovski
Copy link

@davidgiga1993 reported an issue on getDensity() that could potentially break stuff for dpi based layouts and will be fixed here #6263. I'm confident that change will solve his issues without having to modify his code.

@Trurl101 I'm not sure what your issue is but, unless you report what you consider is a bug and be a bit specific we won't be able to help you here. Again, we will happily help you on Discord #ios channel.

I guess the issue @Trurl101 is having is that the units now are less precise when positioning objects on the screen and you would have to manually adjust it with additional computations (to compensate for the loss of precision). If less pixels is better (e.g. 414 vs. 1242), then why do all screen manufacturers always try to increase the number of pixels in the screen? They always strive to increase pixel count and density.

I wonder if it's better to have 414 pixels vs 1242, why are not screens produced with just 414 pixels and why would we ever need screens with 1242 pixels?

@crykn
Copy link
Member

crykn commented Nov 5, 2020

I wonder if it's better to have 414 pixels vs 1242, why are not screens produced with just 414 pixels and why would we ever need screens with 1242 pixels?

I don't think you get the point of this PR.

There is a difference between logical (414x 896) and hardware/physical resolutions (1242x2688). The iOS backend now properly reports those sizes in the same way the four other backends already do. If you require the "real" resolution (1242x2688 in this case), it is as simple as changing getWidth() to getBackBufferWidth().

@obigu
Copy link
Contributor

obigu commented Nov 5, 2020

I guess the issue @Trurl101 is having is that the units now are less precise when positioning objects on the screen and you would have to manually adjust it with additional computations (to compensate for the loss of precision).

You may want to always work with exact integers for positioning your objects and have a 1 world unit - 1 screen pixel, that's ok, but that doesn't mean you lose precision capability (as in the sense of not being able to place stuff with precision) when using a smaller points/logical pixels world size, positions don't have to be integers.

If that's the case, if your project works in a world with a 1 world unit - 1 screen pixel mapping, you can keep the same behaviour as you had before by using the ScreenViewport setUnitsPerPixel() to getBackBufferWidth() / getWidth() without having to change any more lines in your code.

@Trurl101
Copy link
Contributor

If that's the case, if your project works in a world with a 1 world unit - 1 screen pixel mapping, you can keep the same behaviour as you had before by using the ScreenViewport setUnitsPerPixel() to getBackBufferWidth() / getWidth() without having to change any more lines in your code.

Thanks for the hint. I implemented this, and it seems to work.

@JoelOtter
Copy link
Contributor

JoelOtter commented Jan 17, 2021

In the LWJGL3 adapter we can set HdpiMode.Pixels in order to get real pixel values in the resize(int width, int height) event in the ApplicationAdapter, and presumably Gdx.graphics.getWidth(). Would it make sense for this kind of thing to exist on the iOS backend also such that legacy applications can be updated without this breaking?

As it is, it is debatably incorrect to claim that this aligns iOS with the behaviour of the other backends. Android is always raw pixels regardless of screen density, as far as I understand, and LWJGL3 allows one to set the HDPI mode.

I'm happy to attempt to add this HDPI mode to iOS, but would be good to know beforehand if the maintainers are dead against it. :)

@obigu
Copy link
Contributor

obigu commented Jan 19, 2021

@JoelOtter Sure, I think it'd be a good addition!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Label for breaking changes graphics Concerning Gdx.graphics ios
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet