@Sebastian, Ben: Thanks, I pushed the patch here:

http://hg.savannah.gnu.org/hgweb/octave/rev/2da65009cc7f

I'll close this report at comment #170 (!) and thank everyone who participated in bringing this feature into Octave, especially John Swensen. If any issue related to high resolution screens should be noted please start a new report from fresh.

@Pantxo

File #45065 also works for me.

I will now compile default without patch 45059 and see if the problem is already there. Sorry, that I could not test frequently last week.

@Ben: maybe you have a TV screen? That is how I test this at home :)

@Ben: does dragging the window between high-res and low-res screens work for you? I think it does not work here. However, I have to test more carefully.

Seb.

@Ben: I attached an updated cset, which should fix both the object selection and "buttondownfcn", and the scaling of text back-planes.

(file #45059)

@Ben: Cool, that is a starting point. I noticed though that the back plane of text objects where not scaled correctly (that I can reproduce when setting _device_pixel_ratio_ larger than 1). I attached a cset that fixes this for me but could also break the "buttondownfcn". Can you test it?

(file #45032)

Done:
http://hg.savannah.gnu.org/hgweb/octave/rev/d3f5d021255d

@Mike: Yes, John's latest changes provided a way to detect when a window is moved from one screen to another and adapt the rendering accordingly.
Anyway, prior to Qt5 there was no "devicePixelRatio" method in QScreen [1] and the Qt5 behavior of using "logical" pixels versus "physical" pixels was not completely in place, so I don't know if porting this to Qt4 necessary or even feasible (no issue reported against Qt4 builds).


[1] https://doc.qt.io/archives/qt-4.8/qscreen.html#width

@Mike: Was there any change needed to have Qt figures work on Mac with Qt4 (that is not what the title says). If not then the attached patch (that encloses the include statements in the same devicePixelRatio preprocessor conditional) should be enough. Also I don't understand what would be the use of QApplication::primaryScreen.

(file #45030)

This change that has been pushed on the default branch currently breaks building Octave with Qt 4.x. The QWindow class was not introduced until Qt 5. Does anyone have interest in porting this change or making it conditionally compile to work with Qt 4.x?

@Pantxo: With but only as if the axes are within the magenta box. Without the button-downfcn feature does not work.

I've experimented more carefully and adjusted the location of the magenta box. See the new attachment.

The current point displayed is corresponds to what is displayed.

Ok. After more experimenting ... the buttondownfcn works for all objects, but the location/size of the objects detected by buttondownfcn is not the same as what is displayed. I'm attaching a rough estimate of the location of the axes associated with  the buttondownfcn. The magenta box is the area where the buttondownfcn thinks the axes is located.

Yes, in general "buttondownfcn" works for all objects I have tried on linux.
I attached a tentative patch but I can't test it, I have a standard screen. Can someone test if the "buttondownfcn" test from comment #138 (clicking text) works better?

(file #45021)

Thanks Ben, I pushed the patch here:

http://hg.savannah.gnu.org/hgweb/octave/rev/4e108574385c

Marking "ready for test".

Ok, so let's take care of the mouse interactions in a second step.

I attached an updated and documented version of John's patch. The main difference with the previous version is that I removed the explicit call to graphics_toolkit::redraw_figure when the screen changes. Since we change a figure property, the redraw is expected to happen anyway.

Can you check that the figure is actually redrawn when you drag it to/from a HDPI screen?


(file #45016)

@Pantxo,

Although the buttondownfcn doesn't work for me, please push the changeset. I haven't looked over the code for consistency, and its been through a lot of "rebasing" so you may want to review before pushing.

Ben

If the latest patch works, I'll review it, add a change log and push it (in the name of John Swensen) so that we have time to discover any regression before the next major release.

Since I don't have a HDPI screen I'll need some more information:

• Does printing work correctly. Can someone attach the svg file resulting from the following snippet:

demo hold 6
xlabel XLABEL
ylabel YLABEL
axis auto ## needed to avoid bug #48689
print hold6.svg ## may take a while

• Also I am curious to know if mouse operation with HDPI screens work. Are coordinates displayed on the status bar when hovering an axes with the mouse cursor correct? If you click on the text labels in the example below, does Octave display the corresponding text?

plot (1:4);
text (1, 1, "1", "backgroundcolor", "r", "fontsize", 15,...
      "buttondownfcn", @(h) disp (get (h, "string")))
text (2, 2, "2", "backgroundcolor", "r", "fontsize", 15,...
      "buttondownfcn", @(h) disp (get (h, "string")))
text (3, 3, "3", "backgroundcolor", "r", "fontsize", 15,...
      "buttondownfcn", @(h) disp (get (h, "string")))
text (4, 4, "4", "backgroundcolor", "r", "fontsize", 15,...
      "buttondownfcn", @(h) disp (get (h, "string")))

@Ben: yes! The text is not correctly scaled on the high-res screen. A screenshot is attached.

I think that is a regression! It's time that this problem gets fixed and the patch pushed :(

Sorry for the noise. I had a local problem with conflicting patches. I confirm that patch #44985 works.

@maintainers: please merge this into default.

@Ben: sorry, it does not work for me, neither before nor after bootstrap (default branch). If apply the patch after bootstrapping I see rejects for libgui/src/main-window.cc

Sebastian

I applied this patch on hg id f7ef179096ee and cross-compiled for Windows with and without the patch.

I didn't notice any difference when dragging the main window or a figure window from a HiDPI screen to a screen with "normal" DPI and back. Both versions display fine.
Sometimes I need to resize the figure window by a tiny amount to get the axes to adapt to the new window size. But that is not always reproducible.

To sum it up: The patch doesn't break anything for Windows.

HiDPI problem is present also on Linux. Attached screenshot has been taken on Arch Linux with latest version. 4.4. Actually it looks like that GNU Octave is the only program with problems on HiDPI.

Welp, that was a crazy easy fix! It appears that the redraw was getting triggered before the screenChanged signal was triggered. My simple solution was to just request a redraw in the screenChanged event handler.

Just to be safe, I also did a redraw request it at when the windowShown event is triggered and we update the devicePixelRatio in that function.

See the attached diff patch (I really tried this time to get the formatting correct).



(file #44041)

Dear all,

sorry, I was very busy last week. I applied the latest patch (43892) but it still does not rescale the plot correctly when moving from screen A (right) to screen B (left) and backwards. I attach three screenshots ("seb_screenshot_X.png") to demonstrate the problem

1. starting Octave on the normal screen A (right)
2. after moving it to the hidpi screen B (left)
3. after moving it back to the normal screen A (right)

Everything is fine if I stay with the plot window on a hidpi or normal screen, respectively.

Thanks,
Seb.

@Ben: Savannah does have an upload limit which they incorrectly say is 16MB.  Through testing, I have discovered that it is actually 2^21B or ~2MB.  That's why your 4MB file could not be attached.

@Ben: Nice artwork ... besides bug #48689 there is a scaling issue that I think Rik mentionned in one of the 118 comments. I attached an updated patch that should avoid the scaling issue in the resulting figure. Can you confirm? If yes, does this fix have any effect on the framebuffer error you reported?

Rik also mentioned an issue with the text extents which I think I fixed (at least partly) with the attached patch. Does the following look good?

clf
annotation ("textbox", [0 0.5 0.2 0.2], "string",
            "The box should have the same size as the text")

Also, on some linux distros, Qt 5.1 is not enough to get full offscreen rendering support. What does the following return?
 

__octave_config_info__ ("QT_OFFSCREEN")

(file #43892)

The example didn't hang, but did throw an error.

>> figure ("visible", "off")
>> plot (1:10)
>> print bar.pdf
warning: opengl_renderer: Error 'invalid framebuffer operation' (1286) occurred in init_gl_context
warning: called from
    __opengl_print__ at line 158 column 7
    print at line 555 column 14
warning: opengl_renderer: Error 'invalid framebuffer operation' (1286) occurred drawing 'line' object
warning: called from
    __opengl_print__ at line 158 column 7
    print at line 555 column 14

and

  Qt CPPFLAGS:                   -I/usr/local/Cellar/qt/5.10.0_1/include/QtNetwork -I/usr/local/Cellar/qt/5.10.0_1/include -I/usr/local/Cellar/qt/5.10.0_1/include/QtOpenGL -I/usr/local/Cellar/qt/5.10.0_1/include -I/usr/local/Cellar/qt/5.10.0_1/include/QtPrintSupport -I/usr/local/Cellar/qt/5.10.0_1/include -I/usr/local/Cellar/qt/5.10.0_1/include/QtHelp -I/usr/local/Cellar/qt/5.10.0_1/include -I/usr/local/Cellar/qt/5.10.0_1/include/QtWidgets -I/usr/local/Cellar/qt/5.10.0_1/include -I/usr/local/Cellar/qt/5.10.0_1/include/QtGui -I/usr/local/Cellar/qt/5.10.0_1/include -I/usr/local/Cellar/qt/5.10.0_1/include/QtSql -I/usr/local/Cellar/qt/5.10.0_1/include -I/usr/local/Cellar/qt/5.10.0_1/include/QtCore -I/usr/local/Cellar/qt/5.10.0_1/include
  Qt LDFLAGS:                    -F/usr/local/Cellar/qt/5.10.0_1/lib
  Qt libraries:                  -framework QtNetwork -framework QtOpenGL -framework QtPrintSupport -framework QtHelp -framework QtWidgets -framework QtGui -framework QtSql -framework QtCore  -lqscintilla2_qt5
  Qt moc:                        moc
  Qt uic:                        uic
  Qt rcc:                        rcc
  Qt lrelease:                   lrelease
  Qt qcollectiongenerator:       qcollectiongenerator
  Qt qhelpgenerator:             qhelpgenerator

and

  Default pager:                 less
  gnuplot:                       gnuplot

  Build Octave Qt GUI:                  yes (version: 5)
  JIT compiler for loops:               no
  Build Java interface:                 yes
  Build static libraries:               no
  Build shared libraries:               yes
  Dynamic Linking:                      yes (dlopen)
  Include support for GNU readline:     yes
  64-bit array dims and indexing:       yes
  64-bit BLAS array dims and indexing:  no
  OpenMP SMP multithreading:            no
  Truncate intermediate FP results:     yes
  Build cross tools:                    no
  Build docs:                           no

The file foo.pdf is attached

@Pantxo

>> test getframe
PASSES 2 out of 2 tests

I've attached foo.pdf

The script hung the gui on MacOS. There's no osmesa on MacOS, could that be the problem.

@Ben: Could you check how printing works, e.g.:

• Does "test getframe" pass?
• Does "demo ('hold', 6); print ('foo.pdf'); open ('foo.pdf')" look good;
• If the above works, run the attached full demo suite (modified to run invisible). It will take some time to complete (10-15 min on my computer).

On linux all of those still work well with the patch applied.



(file #43879)

I attached a simplified version of the refreshed patch from comment #111. At least it compiles fine and works for me but I have no high resolution screen for testing. Could some of you test it?

(file #43871)

Thanks Ben! I confirm that this patch can be applied to head and fixes the retina scaling issues. However, I like to mention that your latest attachment also includes the anti-freeze patch from #50025.

Sebastian

ugh ... spell checker change "ppfit" to "profit"

It's too late in the release cycle to fit this bug in.  A lot of progress was made and then the bug stalled in February.

Is there any hope that this will make it in time for 4.4?

Sebastian

In the long run, I think we need it to be implemented at the per-figure level, rather than at the renderer level. I don't think there is a different opengl_renderer object for each figure, right? I had been doing it the way you suggest as just a static member variable until we realized this does not work for mixed DPI screens.

An alternative is to just make it a member variable of each Figure C++ class and store it and retrieve it there. My last patch (which apparently has a lot of problems for other people) was an attempt to have a per-figure scaling that got updated when the figure transitioned between screens. I don't care where that information is stored (properties vs. class member), but really think it need to be per-figure.

I have been swamped with my third year review and a ton of other stuff these last couple of weeks. I should have more time to look at it in the coming weeks.

@John: I am currently trying to implement -opengl, -painters and -RGBImage print options and it will be useful to be able to control the resolution of the opengl renderer. In this perspective, it would be handy (and simpler I think) to add a "opengl_renderer::devicePixelRatio" attribute and set its value with a public method "opengl_renderer::set_devicePixelRatio (double)" before rendering. The advantage would be:

• devicePixelRatio is accessible from everywhere in the opengl_renderer class, without having to get it multiple times from figure properties
• the print function can change/restore the value of _devicePixelRatio_ property in order to achieve whatever resolution is required

Do you think it would be possible?

The patch (file #43077) no longer builds for me. I tried fixing the compile errors, but the result hung when I tried a plot.

@rik Thanks for the thorough review. I can fix most of those issues. Here are some answers to your other questions:

1) The per-figure _device_pixel_ratio_ property is only updated when the screenChanged signal is fired. So, I don't think there is a ton of overhead.

2) I don't think the window size problem is due to your version of Linux. I think this has to do with the fact that Linux and Mac handle window size during transition between screens of different DPI. I am investigating this further, but suspect we may need to further distinguish between Mac and non-Mac HiDPI and on Linux machines whether the screen was created on the HiDPI or LoDPI screen. There seems to be some window scaling that occurs at the OS level on Mac, but not on Linux.

I will try a re-work of the patch, and include some debugging output so I can see why @sschoeps Mac machine isn't behaving the same as mine.

@jpswensen: my setup is as follows: macbook with hidpi screen and an external monitor without hidpi capability.

1) you have QT5?
yes, I have qt5, more precisely version 5.10 on macOS via homebrew (I have mentioned that many times in this thread and did not replace it by qt4 in the mean time :)

2) you re-ran the autogen after applying the patch to get the configure changes in there to look for QScreen::devicePixelRatio?

yes, I started from a fresh checkout and recompiled from scratch with patch #43077. The patch is present which is proven by the fact that I see the correct scaling if use the hidpi-screen only. I also confirmed in comment #52 that I see the correct configure message.

Sorry, bad news. The new patch (file #43077) does only work for hidpi-only Macs. However, in my mixed setup (main screen is normal, laptop screen hidpi) the plot does not correctly detect the changes. I made two movies, see attached.

(file #43087)

So I have a pretty minimally invasive example that stores the devicePixelRatio for each figure independently. Here are some highlights of the required changes:

1) graphics.in.h: create a _device_pixel_ratio_ in each figure object (I figure we might as well call it what QT calls it, rather than screen_scale).
2) FigureWindow.h, FigureWindow.cc: Capturing the showEvent so that it can be forwarded on to the Figure class for setting up the detection of screen changes (QT doesn't let you connect to this signal until after the window has actually appeared).
3) Figure.h, Figure.cc: When the window is first shown (from FigureWindow showeEvent) and from connecting to the screenChanged event, we update the _device_pixel_ratio_ in the properties associated with this Figure

This part is working great. Here is the problem I am having that needs input from all of you. Because the devicePixelRatio is now stored in the parent figure property, I need access to the parent figure in each drawing routine in gl-render.cc

If you go look at the opengl_renderer::draw function in gl-render.cc, you see that some of the draw functions takes the properties and the graphics_object (see draw_uipanel and draw_uibuttongroup), but that most only take the properties (see draw_figure, draw_axes, draw_line, etc).

So, my question is whether there is a compelling reason to not follow a pattern where all of these draw_* functions that are called from opengl_renderer::draw take both the properties and the graphics_object. That way, in any draw_* function that needs the devicePixelRation, I can just call
graphics_object fig = go.get_ancestor ("figure");
like is done in opengl_renderer::draw_uipanel.

Thoughts, suggestions, and comments? This will make it so that transitioning windows between screens with different devicePixelRatios will not mess up the plotting.

@rik5 You are correct. I'm not quite sure how I messed up. I had plotted with that patch and without and think I may have swapped the two results when naming the files and thus misinterpreted.

Good news is that I tested this on a Linux machine and the HiDPI issues mentioned here are resolved (for qt toolkit anyways).

I'm uploading an updated version of the patch.  I wrapped all lines to less than 80 characters.  In addition there were other small tweaks.  Octave prefers to keep the header files alphabetized when possible.  Also I modified the indent to more clearly show what is being rounded.  See below

-    vw[2] = octave::math::round (screen_scale_inverse *
-      static_cast<float> (vw[2]));
-    vw[3] = octave::math::round (screen_scale_inverse *
-      static_cast<float> (vw[3]));
-    glOrtho (0, vw[2], vw[3], 0, xZ1, xZ2);
+    vw[2] = octave::math::round (screen_scale_inverse
+                                 * static_cast<float> (vw[2]));
+    vw[3] = octave::math::round (screen_scale_inverse
+                                 * static_cast<float> (vw[3]));
+    glOrtho (0, vw[2], vw[3], 0, xZ1, xZ2);

This is a big step in the right direction, but still not quite ready to be checked in.  Currently on-screen display is accurate, but printing does not work.  Presumably some scaling also needs to happen in gl2ps-print.cc.

The second major task is to go through each graphic primitive and make sure that it works with HiDPI.  This means figure, axes, line, image, patch, surface, text, light, and hggroup.  For extra credit we could test some of the uiXXX functions as well.

For line objects, it appears that there needs to be a scaling factor in the linewidth.

For text objects there is a problem calculating extents.  You can see this with 'demo legend 2', but a more direct test is

close all
ht = text (0.5, 0.5, "Hello World", "backgroundcolor", "r")

For me the extent of the red background layer is much larger than the actual text size.

Offhand, I think image and surface are probably fine.  I ran 'demo light' and didn't see too much weirdness.  hggroup really is only for grouping things so shouldn't be a problem either.


(file #43070)

@Ben: Not a problem.  And the good news is that I think it will be trivial to fix.

@Ben: Could you file a separate bug report about line width?  I'm pretty sure the issue is just conversion.  According to Matlab documentation, "LineWidth" is specified in points.  But, Octave is using the glLineWidth call with no conversion which sets a value in pixels.  Hence, on an average screen Matlab draws a LineWidth 1 object that is 1/72 of an inch.  But Octave would draw a line which is 1/96 of an inch (standard DPI).  Hence Octave's line will be 72/96 or 75% of Matlab's line.  I pulled your images up in GIMP and zoomed in to 2300% and I measure Octave's line at ~4.3 pixels and Matlab's at ~6.1 pixels for a ratio of 70%.

@Ben: There is an internal hardstop in our rendering of lines at, I believe, a linewidth of 6.  This would make Octave's lines about half of Matlab's when using 'LineWidth', 10.

Could you repeat your test, but with a few lower sizes such as 1, 3, and 5?  I think we will match Matlab which means there isn't any bug to resolve.

Opps, in comment #77, the line widths were set to 10 pts.

Mike - ok, I've corrected the indenting.

Regarding a run-time check for HiDPI, at home I use a notebook, but at work I'm accustomed to 2-3 displays. Each of which have different sizes and DPI.

I expect it will be common to display figures on different monitors and that the different monitors may be HiDPI or not. Further, what if a figure on the HiDPI monitor is moved to a conventional monitor, or vice versa.


(file #42982)

Question: Is it possible to have HiDPI detected and handled at run time?

We could apply this as-is now, just knowing that it fixes everything for Mac users and Linux+HiDPI+QT users, but breaks things for Linux HiDPI users who use FLTK. Then we would open another bug report to fix it for Linux+HiDPI+FLTK users.

This would be a numbers game. Almost all MacOS users with newer machines have this problem. I'm not sure how many regular users are of the Linux+HiDPI+FLTK variety.

Ben - a couple fixes to your latest combined patch:

• the commit log lines should also be wrapped to 80 columns
• in the commit log, you can use "acinclude.m4 (OCTAVE_CHECK_FUNC_QSCREEN_DEVICEPIXELRATIO): New macro." at a minimum, or "New macro to test ..." if you want to add a description
• the indent at the #if conditional block is still wrong

Other than that, and the outstanding FLTK issues, looks ok to me.

Setting the scale to 1.0 for FLTK would work for the Linux+HiDPI+FLTK case, but not for the MacOS+HiDPI+FLTK case. If we set it back to 1.0 for FLTK on MacOS, the window would be large and the OpenGL drawing small.

It looks like there is a HiDPI fix in FLTK for 1.4.x snapshots after r12249 (from 2017-06-02), but none of the distributions have switched to this yet.

So, for the time being, a complete fix fo the HiDPI problems across multiple platforms looks like the following:

1) Include the changes to the OpenGL drawing based on the screen_scale as included in the most recent patch
2) Include the changes to the QTHandles ctor to set the screen scale based on the devicePixelRatio as included in the most recent patch.
3) (Not yet done) Figure out how to compute the screen_scale from existing FLTK functions and compute the screen_scale and set it in the FLTK toolkit ctor only for Linux HiDPI machines. It can be left as 1.0 otherwise (even for MacOS HiDPI).
4) (Not yet done, and not sure if needed because it doesn't use OpenGL, right?) Figure out how to compute the screen_scale from existing GnuPlot means and compute the screen_scale and set it in the GnuPlot ctor.

Any suggested changes to this plan of action?

The recent tests show us that we can't do this QT fix without breaking FLTK in some scenarios without also making FLTK changes because the proposed patch makes changes to the low level OpenGL drawing.

We could make this narrowly scoped to only fix QT/FLTK on MacOS and leave HiDPI broken on other platforms, but would prefer to get it all fixed now.

I'm also seeing "error: unexpected exception while parsing input".  I start up with Qt and when I switch to FLTK I get stuck in an infinite loop with "error: unexpected exception while parsing input" being printed to the command window.

@sschoeps I'm using FLTK 1.3.4, as well. According to [*] the correct rendering on macOS is due to the fact that the retina display wraps the original rendering commands autmatically to the higher resolution (if I understood correcly)

[*] https://groups.google.com/forum/#!topic/fltkgeneral/ZZ1G4c8oCd0

@schoeps I am seeing the exact same thing on my Mac when turning on the FLTK backend, both when running the GUI and with the --no-gui option. It is almost as if FLTK is sending a garbage string to the interpreter

@mweitzel So actually it might be that it is the FLTK backend just needs to make the window size be based on the new screen_scale variable. Looking back at your attached images, it seems that the OpenGL drawing size is correct for the FLTK, it is just that the window size is too small.

Let me work up a quick add-on patch to the semi-final one that bpabbott just posted for you to try.

This probably points to the fact that we also need a runtime check for which toolkit is being used to set the screen_scale, not just a compile time check for QT's devicePixelRatio.

I think the problem is that when QTHandles was loaded, it set the screen_scale to something > 1.0.

@mweitzel Can you try running the FLTK test before you run the QT test (or is there some way to ensure the QTHandles is not initialized at startup and only the FLTK backend is initialized)?

The solution here is probably to make sure that each backend sets the screen scale variable when constructed to the value it needs. It seems that value should be:

a) the result of QT's devicePixelRatio for QT and GnuPlot
b) 1.0 for FLTK

Hi John,

I tried your patch on a recent ArchLinux (KDE desktop, Qt 5.10, UHD 3840x2160). See the attached screenshots (generated with the GUI):

• "graphics_toolkit qt" works fine (fonts could be larger, lines could be a bit thicker)
• "graphics_toolkit gnuplot" works find (fonts ARE larger, lines are also a bit thin)
• "graphics_toolkit fltk" IS BROKEN. The figure window is too small - the scaling of the figure is correct.

I had to disable OSMesa during compilation (although I have mesa 17.3.2 installed) because of the following error:

error: _osmesa_print_: Depth and stencil doesn't match, are you sure you are using OSMesa >= 9.0?

I hope this doesn't spoil my results ...

Yes, I confirm that the patch still works and, indeed, I see a nice feature-based configure check :)

checking for QScreen::devicePixelRatio in <QScreen>... yes

Bye
Sebastian

@Mike

I'm not sure how to treat continuation lines (please check). Otherwise, I think the attached changelog includes all suggested changes.

@Sebastian

Please verify I didn't mangle anything when cobbling everything together.

(file #42970)

Please incorporate the attached patch into yours, you'll have to run ./bootstrap or autoreconf after applying the patch.

Please use "#if defined (HAVE_QSCREEN_DEVICEPIXELRATIO)" instead of HAVE_QT5. Also please ensure the #if is always in column 1, not indented with the rest of the code, and the code within the #if block should be indented the same as if the #if weren't there.

The latest patch looks a lot better, thank you.

My few remaining nitpicks:

• your editor is inserting tab characters, please always use spaces to indent, and make sure there are no extra spaces at the end of a line or on a blank line
• "QApplication::screens().at(0)" should be "QApplication::screens ().at (0)", and "devicePixelRatio()" should be "devicePixelRatio ()"

Have you tried using QApplication::primaryScreen ()? Seems simpler and should be the same as QApplication::screens ().at (0), maybe, hopefully?

I am proposing that someone who does know Autoconf (myself and several other maintainers) add a configure test which would add a HAVE_QSCREEN_DEVICEPIXELRATIO conditional macro to the build. This can easily be merged into your patch since it's entirely orthogonal to the source changes.

I would also like to avoid using the Apple brand name "Retina" in the patch and commit message. HiDPI is the generally used term.

@mtmiller I can fix all of those easily except the configure testing stuff. I have no idea how that stuff works, nor the time/desire to figure it out. Can I assume HAVE_QT5 is set or not set as it is already something in the configure process?

Yes the devicePixelRatio is new since QT5, but both QApplication and QScreen are available in QT4.8 (I'm not sure about <4.8).

Fwiw, the compilation succeeds if I strip the ".h" from the failing line. On a non-HiDPI screen, I cannot see any obvious differences between the GUI and plots before and after the patch.

The patch from comment #38 compiled just fine with Qt4 on Ubuntu 17.10. However with Qt5, I get this compile error:

  CXX      libgui/graphics/libgui_graphics_libgui_graphics_la-GLCanvas.lo
../libgui/graphics/GLCanvas.cc:36:12: fatal error: QApplication.h: No such file or directory
   #include <QApplication.h>
            ^~~~~~~~~~~~~~~~
compilation terminated.
Makefile:16539: recipe for target 'libgui/graphics/libgui_graphics_libgui_graphics_la-GLCanvas.lo' failed
make[2]: *** [libgui/graphics/libgui_graphics_libgui_graphics_la-GLCanvas.lo] Error 1
make[2]: *** Waiting for unfinished jobs....

Apart from that you should break some very long lines in your patch. Usually, we break lines at 80 characters.

I attached new patches and changelog files. In the changelog, I also included  bug #52878 as being fixed because I am almost assured it is the same issue. However, we should probably wait to hear back from someone who can build on that Linux machine before we declare it fixed (or a duplicate of this bug).

This also has the casting problems fixed that had been pointed out.

(file #42964, file #42965)

@jpswensen: you think https://savannah.gnu.org/bugs/?52878 is unrelated?

It has been some time since I last compiled with Qt4 but I have the build tree still hanging around. I will try and build with your patch on Ubuntu 17.10.
I don't have a high dpi monitor. Is this important?

Does anyone know which flavors/version of Linux distros still use QT4, so I can test the patch in a virtual machine? It didn't look like there were any takers, but we probably shouldn't push this change until we verify that it doesn't break something for the largest user base.

I will get the ChangeLog text.

Great! I can confirm that plotting, images and colorbars work fine on macOS High Sierra with qt 5.10. We are really close to release octave 4.4 with proper mac support. I am excited :)

Thanks John!

Bye
Sebastian

OK. I reworked the patch to store the information in the display_info object and have the scale default to 1. Only on machines that have QT 5.0 or greater will we query for the screen scale and then set it during the construction of the QtHandles object.

This patch should also take care of the image objects and the colorbar (which is technically also an image).

I am sure there are a whole slew of other OpenGL-drawn objects that might need an update based on this screen scale factor, but I haven't seen them yet.

I also suspect I am in violation of many Octave code style standards. Please let me know what I need to fix and test away!

In particular, it would be nice to have one of the Linux folks with QT4.8 give it a whirl to make sure I didn't break anything.



(file #42946)

@John,

Scripts are attached. I use them to build from my mercurial archive. The configure script sets the prefix to /usr/local/<username>/octave. That way, in the event you do install, Homebrew is safe.

I typically don't worry about installing and run octave via ./run-octave

Ben

(file #42910, file #42911)

@John

Regarding: "I can't ever seem to get the patches and configure parameters right if I try to do it manually." in comment #10.

I have a two shell-scripts for Homebrew on macOS that I used to configure and make Octave (default branch). Is that what you need?

(file #38902) no longer applies to the default branch, so I've rebased it.

(file #42901)

I have no access to macOS.

I am simply offering a review of the patch from my perspective, building Octave on Debian with either Qt 4 or Qt 5.

Yes a read-only internal-use (double-underscore) property in the root graphics object sounds like a good idea to me, that way it could more easily be queried in the interpreter as a side benefit.

The patch is already shipped with the homebrew 4.2 release (with small modifications such that it applies to the devel and release versions) [1]. So, I can confirm that it works.

Sebastian

Sorry to keep posting, but I stink at making patches. This one has all the fixes as the previous patch, but is against the HG tip. The last patch was missing the #include of QApplication.

(file #38902)

That patch I submitted still has big problems. It gets the plot to fill the entire axis, but now the plot coordinates are all messed up.

For example, I tried to
plot(0,0,'ro');
and it puts the marker at (-0.66, 0.58) according to the axis labels.

This is already a huge step forward. Thanks! I am looking forward to include the patch in the 4.2 homebrew formula (since it will be to late for the official release).

Sebastian

P.S. the octave gui freezes on exit when using homebrew's qt5. Do you observe
the sane?

This quick fix for the problem may be as simple as setting the viewport correctly based on the Retina size in pixels.

See http://stackoverflow.com/questions/25230841/how-to-find-display-scaling-factor-on-retina-4k-displays
and
https://forum.qt.io/topic/53059/qt5-and-opengl-with-a-mac-retina-display

I really need to figure out how to get homebrew to build from a local code location. I can't ever seem to get the patches and configure parameters right if I try to do it manually.

Yes, we are trapped: octave with qt5 is not tested well but qt4 is not supported anymore since 2014 (with an extension to 12/2015). It does not compile on recent Macs without ugly hacks. Some package managers dropping support since nobody is officially fixing qt4 (according to homebrew maintainers there are a couple of severe unfixed security issues).

I have actually disabled the GUI in homebrew alltogether because of this :(

Sebastian

I have just upgraded to 4.2.0rc2, qt 5.7.0 on macOS Sierra. The problem is still there and make plotting really annoying. Is anyone working on this? Can I help debugging, testing etc.?

FLTK is not affected, see attached images. Regarding "QWindow::devicePixelRatio ()": I think so too. I added a possible fix for the retina icon bug 46723 [1]. However, the plotting backend is beyond my skills :)

[1] https://savannah.gnu.org/bugs/download.php?file_id=38206

The value is between 149 and 114, see images attached (I used the monitor settings "default" and "more space").  However, in both cases the drawing area is too small by a factor of 2.

Sebastian

I see wrongly scaled plot windows on my Mac when using the native qt5 backend. This might be due to "Retina" dpi settings of qt/Mac. I am not sure if other platform have similar problems? Issue 46723 might be related, [1].

[1] https://savannah.gnu.org/bugs/?46723