Skip to content

Fixes for DPI scaling. Mouse and select events now deliver GL surfac… #262

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

Merged
merged 4 commits into from
Jul 24, 2023

Conversation

gbburkhardt
Copy link
Contributor

Note: Filling out this template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the
maintainer's discretion.

Description of the Change

Fixes for DPI scaling. Mouse and select events now deliver GL surface coordinate values instead of the AWT screen coordinates. Added Windows JOGL jar files that include Fix for AWT GLCcanvas DPI scaling. and Set JAWTWindow:reqPixelScale before the DPI scaling factor is determined and fix AUTOMAX_PIXELSCALE. Noticed that some null pointer exceptions occurred during KMLViewer startup; added null pointer checks.

Why Should This Be In Core?

See Incorrect handling for DPI Scaling. and using 4k, high resolution monitors

Benefits

More high resolutions monitors are in use. Typical DPI scaling settings are no longer 100%. These fixes enable use of those monitors with DPI scaling settings other that 100%. It turns out that the code is cleaner this way. All drawing in the WorldWindow is done using GL surface coordinates. Having mouse event positions changed up front simplifies the application code.

Potential Drawbacks

These changes fundamentally change the notion of screen coordinates to use GL surface coordinates everywhere. It's possible that some places that require changes have been missed.

Applicable Issues

… coordinate values instead of the AWT screen coordinates. Added Windows JOGL jar files that include sgothel/jogl#110 and sgothel/jogl#112.  Noticed that some null pointer exceptions occurred during KMLViewer startup; added null pointer checks.
PJHogan
PJHogan previously approved these changes Apr 8, 2023
PJHogan
PJHogan previously approved these changes May 16, 2023
@markpet49 markpet49 self-requested a review May 31, 2023 04:07
Copy link
Member

@markpet49 markpet49 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've spent a bit of time reviewing this as it's a change that affects a lot of things. So far, it looks pretty good functionally. I haven't found any bugs. Nits: I would like to see the jogl/gluegen source archives removed. The indentation of the source files is off (Example: AbstractSceneController line 115). Also, there is a more recent Jogl release candidate available.
If you don't have time to take care of these items, I will take care of it. Anyhow, I will test some more tomorrow and merge it later this week. Thanks for the taking the time to submit it and fill out the required NASA paperwork.

@PJHogan
Copy link

PJHogan commented May 31, 2023 via email

@gbburkhardt
Copy link
Contributor Author

Thanks for checking it out. Whether you include the Jogl source in your branch is up to you, of course. I always find it useful to have the source code around. But it's available should someone want it.

I did give you pretty recent edition of the Jogl code, from 5/15 ;-) . I see that there's a 5/23 release candidate now. Hard to keep up. I briefly looked at the new changes, and it didn't seem like the new code would affect WorldWind. But since you're updating that code, it's worth getting the latest.

Sorry about the indentation - generally, I try to leave the style of existing code unchanged.

I did try updating the comments appropriately for the change to using surface screen coordinates, but it wouldn't surprise me if I missed some.

@markpet49
Copy link
Member

@gbburkhardt I can take care of the nits that I listed, I think I'll create a branch for your changes in the WWJ repo. In testing today, I noticed that text (annotations, screen text, etc) do not scale appropriately. Also, rotating the globe vertically with the mouse (click and drag) is glitchy. I will investigate, but if you have any thoughts, let me know.

@gbburkhardt
Copy link
Contributor Author

gbburkhardt commented Jun 2, 2023 via email

@gbburkhardt
Copy link
Contributor Author

gbburkhardt commented Jun 3, 2023

I see that the font size for tool tips, the scaleLayer, and other text on the GLCanvas doesn't scale with the overall Windows display scale. Just to be clear, that's what you think should change with the Windows display scale, right? I took a cursory look at WorldWind's text rendering, and noticed in a few places that there is a scale parameter. I'll to review it more, but the current display scale is available from GLCanvas.getMaximumSurfaceScale(). That method's name is a bit of a misnomer, since it returns 'hasPixelScale', which is the current graphics scaling.

Curiously, the CompassLayer, ViewControlsLayer don't scale with DPI scaling change, but WorldMapLayer does. I think that's OK.
The text in the ScaleLayer is not scaling with display scale changes. The size of the measuring portion of the ScaleLayer looks correct. What do you think?

I haven't noticed any lack of smoothness in dragging the globe vertically. Did you notice that in a particular sample application?

BTW, I use Eclipse Adoptium\jdk-11.0.18.10-hotspot.

@gbburkhardt
Copy link
Contributor Author

gbburkhardt commented Jun 4, 2023

I added a fix for TextRenderer, so the text drawn in the GLCanvas are scaled as well. I notice that OnScreenLayerManager is overlaying the ViewControlsLayer, but I think it did that before. I'll be submitting that change to Jogamp.

So why do you have your own copy of TextRenderer, instead of using the one that comes with the Jogl library? I think that changing to the Jogl library edition, or subclassing it, would be better from a maintenance point of view.

@gbburkhardt
Copy link
Contributor Author

Mr. Gothel accepted my proposed fix to the text rendering. Use DPI scaling to scale drawn fonts.

@markpet49
Copy link
Member

@gbburkhardt Thanks for the update, the text looks a lot nicer. The only remaining bug that I've found is that mouse navigation is not fully functional when the display is scaled on my Ubuntu box. Seems to work fine on Windows though. I'm thinking I'll just merge this PR though and log that as a defect to look into later.

There are some other things that might be nice to do as well (scaling things like the compass), but I think we should merge this as it works a whole lot better on scaled displays than the current version. I'm going to do some further testing on Monday and then I'll merge.

Regarding TextRenderer, that was written long before my time with the project. The comments seem to indicate it was created for performance/efficiency reasons.

Thanks for the contribution.

@gbburkhardt
Copy link
Contributor Author

Very good.

I never tested this code on Linux, but I will over the next few weeks. Please copy me on the bug report, and provide more details on what isn't working properly.

I didn't supply JOGL native libraries for Linux in that PR. It shouldn't be too difficult for you to add them. I haven't checked to see if there's a new release candidate available on jogamp.org after my fix for the text scaling went in. If you are building the code yourself, it turns out that the JOAL library is now needed to build JOGL and GlueGen.

After thinking about it a bit, I'm not sure that the compass, view controls, or world map should be scaled. Perhaps there should be a user accessible control to set their size as a proportion of the viewport.

I'm pretty sure that the TextRenderer class in WorldWind is the same as supplied with the JOGL library, except for exposing a debug flag. I'll probably post another pull request to take it out. I think it's better to rely on jogamp.org to maintain that code.

…9e, jogl commit 900c35c6a. Includes fix for jogl version of TextRenderer. Added Linux native jar files.
Copy link
Member

@markpet49 markpet49 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still some warts, but overall a good change, merging this to make some adjustments.

@markpet49
Copy link
Member

Well, I guess I'll merge it when I figure out how to disable Travis....

@markpet49 markpet49 merged commit 55213aa into NASAWorldWind:develop Jul 24, 2023
@thwe74
Copy link

thwe74 commented Oct 31, 2023

Scaling of the Map works but I have a question for this new implementation of getMousePosition. In the example "ContextMenusOnShapes" the pop-up does not open any more at the mouse pointer position . This happens on Windows 10/11 and Ubuntu Linux. How can the pop-up menu shown at cursor position? How can the GL Mouse Coordinates translated into pixel coordinates?
grafik

quonn77 added a commit to quonn77/WorldWindJava that referenced this pull request Apr 23, 2024
From https://github.com/WorldWindEarth/WorldWindJava/commits/develop/

We got the following commits (partial lists):

57b265b
4afe979
3388e26
e45b49d
a4bf62d
56a90c4
f4c1734
2f92b3e
6225f20
e8262dc
9e6e504
81731e8
843d22f

From https://github.com/NASAWorldWind/WorldWindJava/commits/develop/

We got the following commits

 - Fix display of context menu in ContextMenusOnShapes demo. (NASAWorldWind#274)
 - Delete local copy of worldwind/render/TextRenderer in favor of version that is maintained by JOGAMP. Update JOGL jar files with released version 2.5.0, build Build: 2.5-b1533-20230818. That build i…
- Implementing formatting rules. (NASAWorldWind#271)
- New JOGL to fix linux crashes (NASAWorldWind#270)
- Remove extraneous source (NASAWorldWind#269)
- Fixes for DPI scaling. Mouse and select events now deliver GL surfac… (NASAWorldWind#262)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants