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

[BUG] - MultiViewer is broken on Linux with a hidpi monitor #179

Open
r3dl3g opened this issue Mar 6, 2024 · 13 comments
Open

[BUG] - MultiViewer is broken on Linux with a hidpi monitor #179

r3dl3g opened this issue Mar 6, 2024 · 13 comments
Labels
bug Something isn't working

Comments

@r3dl3g
Copy link

r3dl3g commented Mar 6, 2024

Describe the bug
On a hidpi monitor (were dpi_scaling is 2) the MultiViewer doesn't display as expected

Tell us how to reproduce the bug
Just run the demo "Tutorial_205_MultiView" on a hidpi monitor.

Development/Running environment:

  • OS: Linux (Nixos 23.11)
  • Compiler gcc (GCC) 12.3.0
  • Debug or release: Both

Additional context
The dpi_scaling is unnecessary applied to the sub viewports.

Here's the diff for my patch to fix it:

diff --git a/easy3d/viewer/multi_viewer.cpp b/easy3d/viewer/multi_viewer.cpp
index 3116de7d..d7f943ba 100644
--- a/easy3d/viewer/multi_viewer.cpp
+++ b/easy3d/viewer/multi_viewer.cpp
@@ -222,14 +222,16 @@ namespace easy3d {
 
         for (int i = 0; i < num_rows_; ++i) {
             auto &row = views_[i];
-            const auto y = height() - (i + 1) * view_height_;
-            for (int j = 0; j < num_cols_; ++j)
+            const auto y = i * view_height_;
+            for (int j = 0; j < num_cols_; ++j) {
+                const auto x = j * view_width_;
                 row[j].viewport = ivec4(
-                        static_cast<int>(static_cast<float>(j * view_width_) * dpi_scaling()),
-                        static_cast<int>(static_cast<float>(y) * dpi_scaling()),
-                        static_cast<int>(static_cast<float>(view_width_) * dpi_scaling()),
-                        static_cast<int>(static_cast<float>(view_height_) * dpi_scaling())
+                        static_cast<int>(static_cast<float>(x)),
+                        static_cast<int>(static_cast<float>(y)),
+                        static_cast<int>(static_cast<float>(view_width_)),
+                        static_cast<int>(static_cast<float>(view_height_))
                 );
+            }
         }
 
         // ------------------------------------------------------------
@r3dl3g r3dl3g added the bug Something isn't working label Mar 6, 2024
@LiangliangNan
Copy link
Owner

Thanks for reporting the potential issue.
I don't have such a device for debugging, which makes it tricky for me.
Can you attach some snapshots?

@LiangliangNan
Copy link
Owner

By checking your code, I found that you have changed
const auto y = height() - (i + 1) * view_height_;
to
const auto y = i * view_height_;
This seems wrong because the OpenGL tradition is to have the origin of the coordinate system at the lower left corner. But the window system (based on GLFW) has that at the top left corner.
I still believe the dpi_scaling should be applied, so I guess your change to the calculation of the Y coordinate is the source of the issue. Can you please check?

@r3dl3g
Copy link
Author

r3dl3g commented Mar 7, 2024

Of course,

Without the patch:
Without the patch

With the patch:
With the patch

@r3dl3g
Copy link
Author

r3dl3g commented Mar 7, 2024

By checking your code, I found that you have changed
const auto y = height() - (i + 1) * view_height_;
to
const auto y = i * view_height_;
This seems wrong because the OpenGL tradition is to have the origin of the coordinate system at the lower left corner. But the window system (based on GLFW) has that at the top left corner.

Ah, I didn't was aware of this OpenGL tradition. So sorry for this unnecessary change.

@r3dl3g
Copy link
Author

r3dl3g commented Mar 7, 2024

So I changed this part back and that's the result:
Bildschirmfoto vom 2024-03-07 11-11-34

And that's the diff now:

diff --git a/easy3d/viewer/multi_viewer.cpp b/easy3d/viewer/multi_viewer.cpp
index 3116de7d..1eeb59cf 100644
--- a/easy3d/viewer/multi_viewer.cpp
+++ b/easy3d/viewer/multi_viewer.cpp
@@ -223,13 +223,15 @@ namespace easy3d {
         for (int i = 0; i < num_rows_; ++i) {
             auto &row = views_[i];
             const auto y = height() - (i + 1) * view_height_;
-            for (int j = 0; j < num_cols_; ++j)
+            for (int j = 0; j < num_cols_; ++j) {
+                const auto x = j * view_width_;
                 row[j].viewport = ivec4(
-                        static_cast<int>(static_cast<float>(j * view_width_) * dpi_scaling()),
-                        static_cast<int>(static_cast<float>(y) * dpi_scaling()),
-                        static_cast<int>(static_cast<float>(view_width_) * dpi_scaling()),
-                        static_cast<int>(static_cast<float>(view_height_) * dpi_scaling())
+                        static_cast<int>(static_cast<float>(x)),
+                        static_cast<int>(static_cast<float>(y)),
+                        static_cast<int>(static_cast<float>(view_width_)),
+                        static_cast<int>(static_cast<float>(view_height_))
                 );
+            }
         }
 
         // ------------------------------------------------------------

@r3dl3g
Copy link
Author

r3dl3g commented Mar 7, 2024

Btw thanks for this wonderful piece of code.
I've been looking for a real high level 3D library for a long time, without the need for masses of glx boilerplate code and then luckily I found this one.

@LiangliangNan
Copy link
Owner

Thanks for your kind words!
I believe the issue is in some other part(s) of the code. For now, you can use your workaround solution.
I cannot debug it because I don't have such a device, but I will check with some friends if can further investigate this issue.
Can you change, in the main.cpp file, from
initialize();
to
initialize(true);
Then you will see a log file Tutorial_205_MultiView.log will be generated next to the executable. I would be grateful if you post the entire log here, in the hope that I can find some clue about the issue.

@r3dl3g
Copy link
Author

r3dl3g commented Mar 7, 2024

Of course. Here there are:

Without patch:
Tutorial_205_MultiView.log

With patch:
Tutorial_205_MultiView-with_patch.log

As far as I can see, they are identical.

@LiangliangNan
Copy link
Owner

I don't have a Linux machine with a hidpi screen. But I've tried to fix this issue using a Mac (which has hidpi).
The new code is in the 'hidpi' branch: https://github.com/LiangliangNan/Easy3D/tree/hidpi
Can you check if it works and post a snapshot here?

@r3dl3g
Copy link
Author

r3dl3g commented Mar 16, 2024

grafik

Sorry, no change on my machine with that fix.

@LiangliangNan
Copy link
Owner

Okay. Then please go with your changes. I will have to find a proper machine to further explore the issue.

@r3dl3g
Copy link
Author

r3dl3g commented Mar 18, 2024

Just for completeness, I testet now also under windows10 on the same machine and got the same results, as under linux:

Original:
MultiView_original

With Patch:
MultiView_patched

@r3dl3g
Copy link
Author

r3dl3g commented Mar 18, 2024

I forgot to mention a very small change to compile it with Visual Studio 2017:

diff --git a/easy3d/util/console_style.cpp b/easy3d/util/console_style.cpp
index d2d793ee..2a5632f3 100755
--- a/easy3d/util/console_style.cpp
+++ b/easy3d/util/console_style.cpp
@@ -30,6 +30,7 @@
 #include <vector>
 #include <cstring>
 #include <cstdlib>  // for std::getenv
+#include <string>
 
 namespace easy3d
 {

But that's does not affect this issue

@LiangliangNan LiangliangNan changed the title [BUG] - MultiViewer is broken on hidpi [BUG] - MultiViewer is broken on Linux with a hidpi monitor Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants