Fixed transform control example

[ahcorde]

I found an issue in the rendering examples, in particular in the transform_control example. When I try to click in the arrows or in one of the other Gizmo visuals is not getting the visual. I need to click in other part of the screen. See the gif below.

With this fix I'm able to get the right visual:

This is also happening in ign-gazebo.

Signed-off-by: ahcorde [email protected]

[codecov]

Codecov Report

Merging #221 (c10a6bb) into ign-rendering3 (5164539) will decrease coverage by 0.04%.
The diff coverage is 37.50%.

@@                Coverage Diff                 @@
##           ign-rendering3     #221      +/-   ##
==================================================
- Coverage           53.23%   53.18%   -0.05%     
==================================================
  Files                 131      131              
  Lines               12007    12027      +20     
==================================================
+ Hits                 6392     6397       +5     
- Misses               5615     5630      +15     

Impacted Files	Coverage Δ
ogre/src/OgreCamera.cc	0.00% <0.00%> (ø)
src/Utils.cc	29.72% <34.61%> (-5.57%)	⬇️
ogre2/src/Ogre2Camera.cc	87.06% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5164539...c10a6bb. Read the comment docs.

[iche033]

could be related to gazebosim/gz-sim#147

[ahcorde]

@iche033 I completely changed the PR, do you mind to have another look ?

[iche033]

interesting, so you have different xScale and yScale values?

[iche033]

The changes look good to me. I'm not able to reproduce this issue on my laptop though.

@jennuine do you still have this issue? If so, do you mind testing to see if this fixes the problem?

[ahcorde]

interesting, so you have different xScale and yScale values?

Yes, I have xScale = 80 and yScale 81,5.

But I think the main issue was that if the scale is lower than the DPI then no scale is needed

[jennuine]

I still have this issue and unfortunately the changes did not fix the problem for me

[ahcorde]

Thank you for testing it @jennuine, can you remove this comment ?

https://github.com/ignitionrobotics/ign-rendering/blob/53e850f0f71e9521fcdc59bec492baa437710c9f/src/Utils.cc#L151-L153

And post the output here.

[jennuine]

DPI Desktop: 96.000000, DPI XY: [162.019928, 160.891495], Scale XY: [1.687708, 1.675953]

[peci1]

I think the code you're searching for Windows is:

#include <windows.h>
#include <GL/gl.h>
// windows.h defines these macros which break other parts of the library
#undef max
#undef min

 float xDpiDesktop = GetDeviceCaps(wglGetCurrentDC(), LOGPIXELSX);
 float xPixels = GetDeviceCaps(wglGetCurrentDC(), HORZRES);
 float xMM = GetDeviceCaps(wglGetCurrentDC(), HORZSIZE);
 float yDpiDesktop = GetDeviceCaps(wglGetCurrentDC(), LOGPIXELSY);
 float yPixels = GetDeviceCaps(wglGetCurrentDC(), VERTRES);
 float yMM = GetDeviceCaps(wglGetCurrentDC(), VERTSIZE);

I did a test on my computer with screen scaling set to 125%, with a FullHD 21,5" display:

xDpiDesktop = 120
xPixels = 1920
xMM = 477

I think it corresponds to the values you're trying to get in X11.

However, the transform control example works for me even without this patch on Windows.

Docs here: https://docs.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-getdevicecaps . The GetDeviceCaps method returns some "generic" DPI common to all screens, but I think it might be better than nothing.

You can also use GetDpiForWindow(hwnd) which gets you DPI for a specific window, which might be better. But I haven't found a good way to access the window handle of the OGRE window. There is Ogre::RenderTarget::getCustomAttribute("WINDOW", &hwnd), but it would need some more logic for different render engines and OSes. However, it might be useful even in case of X11 because that would return you a handle to the actual window and its screen, and not always screen 0.

Also, the current implementation of the added screenScalingFactor(xScale, yScale) function should default to returning ones for unsupported operating systems, not "nothing" as it does now.

[chapulina]

Another reference point:

• Ubuntu Bionic Docker container (host is Ubuntu Focal)
• Display resolution: 2560x1600, scale: 100%
• Output: DPI Desktop: 96.000000, DPI XY: [100.968941, 101.094528], Scale XY: [1.051760, 1.053068]

[chapulina]

Return is void 😉

[chapulina]

I believe this issue has been manifesting on CI, see #170. The VisualAt test has been "flaky", but actually it consistently passes / fails according to which machine it runs on.

[chapulina]

I pushed some tweaks in 58a0857:

• Remove duplicate code
• Break down the math to make it easier to debug

---

I also tracked down the history of the scaling factor.

• It was introduced in this BitBucket PR to support high DPI displays. Before that, the scaling was always 1.0.
• Gazebo classic has APIs to set the scaling factor, but that's never called internally, so it just uses the default 1.0.

With that in mind, I tried setting the scaling to 1.0 and voilà, it worked like a charm.

I don't really understand why the resolution we get from the calculation is different from the one reported by Xrm and how this scaling changes for different hardware, so it's hard for me to make a recommendation here. The fact that Gazebo classic has been using 1.0 so far is a strong indication for me that reverting to 1.0 may be a good option.

---

So I see 3 different setups in this thread:

• @ahcorde 's is fixed with the current PR
• @jennuine 's doesn't work with or without the PR
• @chapulina 's doesn't work with or without the PR, but works with 1.0

@jennuine , @ahcorde : can you try 1.0 / 1.0 scaling just for fun?

I'd also be interested in seeing if @nkoenig could test the laptop that gave origin to the scaling in the first place.

---

https://github.com/osrf/buildfarmer/issues/207

[ahcorde]

it's working for me,

I retarget this to ign-rendering3, because ign-rendering4 it's no Citadel or Edifice.

[jennuine]

It's working for me with or without the PR now... With the PR, I tested both as is and with scaling 1.0 and it works either way. Without the PR, I've tested this on Citadel, Dome, and Edifice and all work as expected now.

🤔 I don't know what could have changed between now and then? Maybe @mabelzhang can give it a try, I believe she had come across this issue awhile ago too.

[chapulina]

it's working for me

@ahcorde , you mean with this PR or with 1.0 or both?

[ahcorde]

I only tried 1.0

[chapulina]

I have a suggestion in #381 for a way to let each user calibrate their screens independently.

[chapulina]

I'm closing this in favor of #446, which was merged into Fortress. We can backport that if needed.