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

[Patch] Add approximation for display DPI on iOS #3465

Closed
SDLBugzilla opened this issue Feb 11, 2021 · 0 comments
Closed

[Patch] Add approximation for display DPI on iOS #3465

SDLBugzilla opened this issue Feb 11, 2021 · 0 comments

Comments

@SDLBugzilla
Copy link
Collaborator

SDLBugzilla commented Feb 11, 2021

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: 2.0.10
Reported for operating system, platform: iOS (All), iPhone/iPod touch

Comments on the original bug report:

On 2019-11-30 23:22:54 +0000, Aaron Barany wrote:

Created attachment 4079
Approximates DPI for iOS.

There appears to be no way to directly access the display DPI on iOS, so as an approximation the DPI for the iPhone 1 is used as a base value and is multiplied by the screen's scale. This should at least give a ballpark number for the various screen scales. (based on https://stackoverflow.com/questions/25756087/detecting-iphone-6-6-screen-sizes-in-point-values it appears that both 2x and 3x are used)

On 2019-12-01 19:12:42 +0000, Alex Szpakowski wrote:

What situations are you using this for?

On 2019-12-02 00:43:36 +0000, Aaron Barany wrote:

Sizing of elements in screen space so they are consistent size and adjusting LOD. At least in these cases being off slightly isn't a big deal, but being off by a factor of 2 or 3 is.

On 2019-12-02 00:48:51 +0000, Alex Szpakowski wrote:

Isn't that kind of what the DPI-scaled units that iOS uses are for (which SDL uses for window sizes and such, on macOS too)?

For example, the iPhone 5s' pixel dimensions are 640×1136, but the OS scales all content by 2x to maintain consistent sizing and positioning of UI elements with other iOS devices while having a different pixel density.
So the OS - and SDL - reports the window size as 320x568, and the actual pixel dimensions can be obtained via SDL_GL_GetDrawableSize or SDL_GetRendererOutputSize (and you can divide the pixel width by the DPI-scaled window width to get the scale factor if you really need).

On 2019-12-02 01:17:54 +0000, Aaron Barany wrote:

Sort of, the code I have uses a reference DPI to adjust the LOD (such as for curve tessellation in screen-space). In order to have this work on iOS with the suggested method, I would need to have a separate code path to compare the two sizes, and also have knowledge of DPI the scale factors are relative to. (165 in this case) It also requires that a window is already created and available in the code that computes the DPI, rather than simply knowing about the screen.

It would be more convenient for higher-level code and require less platform-specific knowledge to have a reasonable approximation of the DPI through the same API as other platforms.

On 2019-12-04 06:12:46 +0000, Sam Lantinga wrote:

It seems like we can use the scale factor and the actual pixel reported by the drawable size to get the real DPI. Is that right? If so, that seems like it would be an improved implementation.

On 2019-12-04 07:14:40 +0000, Aaron Barany wrote:

Unfortunately no, since the resolution doesn't necessarily correspond to the physical dimensions of the screen. For example, an iPad may have the same pixel density (or DPI) as an iPhone, but have a higher resolution due to the larger screen. This is also the case between normal and "+" versions the iPhone. Unless you have a table of specific models, which you'd have to update at least once a year, you don't know the physical dimensions of the screen to find the true DPI.

On 2019-12-04 13:52:57 +0000, Sam Lantinga wrote:

Having a table of models isn't a terrible idea, and unknown models could fall back to some reasonable value...

On 2019-12-08 06:04:02 +0000, Aaron Barany wrote:

Created attachment 4092
Updated DPI calculation

I have updated the patch to use a table of current devices and use a computation as a fallback. I have also updated the fallback computation to be more accurate.

On 2019-12-08 19:36:58 +0000, Sam Lantinga wrote:

Looks good, thanks!
https://hg.libsdl.org/SDL/rev/37350f1e7902

On 2019-12-15 21:02:27 +0000, Aaron Barany wrote:

Created attachment 4100
SDL_DisplayData-constructor-fix.patch

I realized I made a minor mistake in my patch: I changed the constructor prototype for SDL_DisplayData, but didn't update the declaration in the .h file. The compiler and linker don't complain, but it would probably be best to fix in case a later change runs into a problem from the mismatch. I have attached a patch to fix this.

On 2019-12-16 18:25:58 +0000, Sam Lantinga wrote:

Fixed, thanks!
https://hg.libsdl.org/SDL/rev/7324ce6b653b

On 2020-03-28 19:42:01 +0000, David Ludwig wrote:

This is not functional, as far as I can tell. It looks like the UIKit SDL_VideoDevice (which is internal to SDL) is not getting its GetDisplayDPI field set.

The fix is pretty simple (add 'device->GetDisplayDPI == UIKit_GetDisplayDPI;' to UIKit_CreateDevice; I'll attach a patch). Might this have been left out for any particular reason, or is this a bug?

On 2020-03-28 19:47:37 +0000, David Ludwig wrote:

Created attachment 4282
connect SDL_GetDisplayDPI to UIKit_GetDisplayDPI

On 2020-03-28 19:48:13 +0000, David Ludwig wrote:

Also, apologies if reopening this case, rather than opening a new one, is problematic. I can move this to another case if need be.

On 2020-03-28 23:51:18 +0000, Ryan C. Gordon wrote:

(In reply to David Ludwig from comment # 14)

Also, apologies if reopening this case, rather than opening a new one, is
problematic. I can move this to another case if need be.

This is right way.

(Also, go ahead and push that patch if you're ready!)

--ryan.

On 2020-03-29 15:19:10 +0000, David Ludwig wrote:

My extra patch is in at https://hg.libsdl.org/SDL/rev/304c6020419a

On 2020-03-29 23:21:25 +0000, Aaron Barany wrote:

Might this have been left out for any particular reason, or is this a bug?

This was a mistake: I was working with multiple patches, and this change got dropped by accident when I submitted this component of it. Thanks for catching it.

On 2020-04-05 16:32:01 +0000, David Ludwig wrote:

No worries, and thanks for putting this feature together!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant