bugGNU Octave - Bugs: bug #49053, figure scaling issue with Qt 5 on...

 
 

bug #49053: figure scaling issue with Qt 5 on macOS with Retina / HiDPI scaling

Submitter:  Sebastian <sschoeps>
Submitted:  Sat 10 Sep 2016 03:51:42 PM UTC
   
 
Category:  Plotting with OpenGL Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 21 Sep 2018 10:02:10 AM UTC, comment #170: 

@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 Diribarne <pantxo>
Group Member
Fri 21 Sep 2018 09:28:26 AM UTC, comment #169: 

@Pantxo

Indeed the screen change works again with #45065. So the redraw seems necessary. Maybe you should add a comment into the source code to avoid people "optimizing" this again in the future again :)

Sebastian <sschoeps>
Fri 21 Sep 2018 03:11:42 AM UTC, comment #168: 

@Pantxo

File #45065 also works for me.

Ben Abbott <bpabbott>
Group Member
Thu 20 Sep 2018 07:23:12 PM UTC, comment #167: 

@Sebastian: As I said in comment # I have removed a call to graphic_toolkit::redraw_figure fromJohn's patch because I assumed changing the property would be enough to trigger the redraw. Maybe this was a wrong assumption.

I attached a patch in which I added this back.

(file #45065)

Pantxo Diribarne <pantxo>
Group Member
Thu 20 Sep 2018 06:54:36 PM UTC, comment #166: 

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 :)

Sebastian <sschoeps>
Thu 20 Sep 2018 06:43:44 PM UTC, comment #165: 

@Sebastian: I don't have a low res monitor to test on.

Ben Abbott <bpabbott>
Group Member
Thu 20 Sep 2018 03:17:28 PM UTC, comment #164: 

@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.

Sebastian <sschoeps>
Thu 20 Sep 2018 02:17:42 PM UTC, comment #163: 

@Pantxo, file #45059 works for me.

Ben Abbott <bpabbott>
Group Member
Thu 20 Sep 2018 07:28:25 AM UTC, comment #162: 

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

(file #45059)

Pantxo Diribarne <pantxo>
Group Member
Wed 19 Sep 2018 03:29:39 AM UTC, comment #161: 

@Pantxo,

You expectation is correct. The back plane of the text objects is now scaled correctly, but the buttondownfcn no longer works correctly. Same behavior as before.

Ben Abbott <bpabbott>
Group Member
Tue 18 Sep 2018 08:09:32 AM UTC, comment #160: 

@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)

Pantxo Diribarne <pantxo>
Group Member
Tue 18 Sep 2018 01:29:11 AM UTC, comment #159: 

@pantxo,

You should be more bold/confident! File #45026 works for me!

Ben Abbott <bpabbott>
Group Member
Mon 17 Sep 2018 07:21:58 PM UTC, comment #158: 
Pantxo Diribarne <pantxo>
Group Member
Mon 17 Sep 2018 07:08:49 PM UTC, comment #157: 

Makes sense, thanks for clarifying. Can you go ahead and push your change to make the includes conditional to resolve the build error?

Mike Miller <mtmiller>
Group Member
Mon 17 Sep 2018 06:50:54 PM UTC, comment #156: 

@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

Pantxo Diribarne <pantxo>
Group Member
Mon 17 Sep 2018 06:32:32 PM UTC, comment #155: 

Thanks Pantxo, yes your suggested patch to make the includes conditional looks like it will probably solve the problem.

I was suggesting the attached change instead, but maybe the screen object needs to be the screen that the Octave window is actively on, not the main screen, is that right? I was suggesting to simplify and remove the need for the QWindow object at all.

I don't think anyone is trying to use Octave with Qt 4 on macOS now, so I don't think it matters until someone cares to look at it. I'm just looking to make sure Octave still builds with Qt 4 at all. I think it's obvious we should encourage people to use the latest version of Qt 5 available for best results.

(file #45031)

Mike Miller <mtmiller>
Group Member
Mon 17 Sep 2018 06:18:10 PM UTC, comment #154: 

@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)

Pantxo Diribarne <pantxo>
Group Member
Mon 17 Sep 2018 04:50:14 PM UTC, comment #153: 

Ben - in comment #71 I suggested using the QApplication::primaryScreen static method, but that suggestion was not used in the patch that was pushed on default. Can you test whether that still works for you? And please remove the '#include <QWindow>' from Figure.cc. It's the #include that is breaking compilation under Qt 4, even though the devicePixelRatio preprocessor conditional is false.

Mike Miller <mtmiller>
Group Member
Mon 17 Sep 2018 04:36:39 PM UTC, comment #152: 

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?

Mike Miller <mtmiller>
Group Member
Mon 17 Sep 2018 08:22:39 AM UTC, comment #151: 

@Ben: Thanks for the clarifications. Could you try the attached cset (again this is a long shot since I can't test myself)

(file #45026)

Pantxo Diribarne <pantxo>
Group Member
Sun 16 Sep 2018 10:04:03 PM UTC, comment #150: 

@Pantxo: With applied the buttondownfnc works but only as if the axes are within the magenta box. Without applied the buttondownfnc works 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.


Ben Abbott <bpabbott>
Group Member
Sun 16 Sep 2018 07:15:05 PM UTC, comment #149: 

@Ben: IIUC the magenta box is the area where you need to click if you want the "buttondownfcn" of any object (not only text) to be evaluated. Is this correct?
Is this with the patch applied?
Does the patch improve anything or make it worse (the area looks wrong by a factor of 4)?

Also could you check that the "currentpoint" axes property returns sensible results when you click inside axes?


clf
hax = axes ();
addlistener (hax, "currentpoint", @(hax) disp (get (hax, "currentpoint")(1,1:2)))
grid on


Pantxo Diribarne <pantxo>
Group Member
Sun 16 Sep 2018 04:27:37 PM UTC, comment #148: 

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.


Ben Abbott <bpabbott>
Group Member
Sun 16 Sep 2018 04:19:14 PM UTC, comment #147: 

With file #45021 the buttondownfnc works for figures, axes, and line objects. But does not work for text objects. I'm running macOS with an HDMi display. So, this may be a macOS or an HDMi problem.

Is it possible that the buttondownfcn functionality relies on clicking a very small text object? I'll experiment further.


Ben Abbott <bpabbott>
Group Member
Sun 16 Sep 2018 03:58:19 PM UTC, comment #146: 

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)

Pantxo Diribarne <pantxo>
Group Member
Sat 15 Sep 2018 10:45:15 PM UTC, comment #145: 

Thanks you Pantxo,

Pull and update was clean for me on macOS.

btw, for the buttondownfcn test, did that work for you? If so what OS and what version of Qt?

Ben

Ben Abbott <bpabbott>
Group Member
Sat 15 Sep 2018 10:27:25 PM UTC, comment #144: 

Thanks Ben, I pushed the patch here:

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

Marking "ready for test".

Pantxo Diribarne <pantxo>
Group Member
Sat 15 Sep 2018 04:21:35 PM UTC, comment #143: 

@Pantxo,

Changeset #45016 works as expected for me (macOS using Homebrew)

Ben

Ben Abbott <bpabbott>
Group Member
Sat 15 Sep 2018 10:07:14 AM UTC, comment #142: 

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 Diribarne <pantxo>
Group Member
Sat 15 Sep 2018 08:52:04 AM UTC, comment #141: 

I can confirm what Ben said. It's working, clicking has no effect.

Thank you Pantxo!

Sebastian <sschoeps>
Fri 14 Sep 2018 02:30:35 PM UTC, comment #140: 

@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

Ben Abbott <bpabbott>
Group Member
Fri 14 Sep 2018 02:16:58 PM UTC, comment #139: 

Coordinates display on the status bar, but the clicking has no effect.

SVG is attached.

(file #45012)

Ben Abbott <bpabbott>
Group Member
Fri 14 Sep 2018 01:19:33 PM UTC, comment #138: 

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")))


Pantxo Diribarne <pantxo>
Group Member
Fri 14 Sep 2018 10:33:20 AM UTC, comment #137: 

I attached a patch that fixes the regression for me.

(file #45009)

Pantxo Diribarne <pantxo>
Group Member
Fri 14 Sep 2018 07:18:32 AM UTC, comment #136: 

@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 :(


Sebastian <sschoeps>
Fri 14 Sep 2018 03:48:58 AM UTC, comment #135: 

@Sebastian,

I just noticed the fontsize does not render correctly. Can you confirm?

Ben Abbott <bpabbott>
Group Member
Wed 12 Sep 2018 10:10:06 PM UTC, comment #134: 

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

@maintainers: please merge this into default.

Sebastian <sschoeps>
Wed 12 Sep 2018 02:47:30 PM UTC, comment #133: 

@Sebastian: I don't see any changes to main-window.cc in file #44985.

The "wait on worker thread" macOS bug has a change to main-window.cc. But that one also applies cleanly for me. I recall this patch needed a slight modification back in April. The modified patch is present in the other bug report (file #43807)


Ben Abbott <bpabbott>
Group Member
Wed 12 Sep 2018 02:33:02 PM UTC, comment #132: 

@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

Sebastian <sschoeps>
Wed 12 Sep 2018 03:26:36 AM UTC, comment #131: 

Recent commits broke the patch. I'm attaching a modified patch that works for me on macOS.

(file #44985)

Ben Abbott <bpabbott>
Group Member
Tue 11 Sep 2018 04:07:18 PM UTC, comment #130: 

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.

Markus Mützel <mmuetzel>
Group administrator
Fri 07 Sep 2018 09:13:38 AM UTC, comment #129: 

Now with homebrew providing a way to get the octave GUI natively, it would be very nice if this patch could be put into master! It is currently being applied by hand in the Octave app bundler project (https://github.com/octave-app/homebrew-octave-app/blob/master/Formula/octave-octave-app_4.4.0.rb)

Henry Schreiner <henryiii>
Tue 15 May 2018 07:05:24 PM UTC, comment #128: 

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.


Petr Kopecky <kejpi>
Fri 27 Apr 2018 01:17:02 PM UTC, comment #127: 

Great! I can gladly confirm that the patch works. However, I will do some more test w.r.t. legends, axes labels etc.

Best
Sebastian

Sebastian <sschoeps>
Fri 27 Apr 2018 12:50:03 AM UTC, comment #126: 

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)

John Swensen <jpswensen>
Fri 20 Apr 2018 07:59:37 PM UTC, comment #125: 

Good new, everybody!

I can now replicate the behavior that @sschoeps is seeing. I can make it happen even on my two (differing DPI) HiDPI screens on my Mac by going in and changing System Preferences to have both of them as scaled displays with the "More Space" setting.

At least now I can debug a little bit and will check back once I have some insights.

John Swensen <jpswensen>
Tue 17 Apr 2018 08:52:35 AM UTC, comment #124: 

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.


Sebastian <sschoeps>
Thu 12 Apr 2018 03:45:01 PM UTC, comment #123: 

I noted in one of the earlier comments that the text extents were not calculated correctly.  The test code I used was


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


In this case the text is rendered perfectly, but the background rectangle which depends on the calculation of text extents is much too large.



Rik <rik5>
Group administrator
Thu 12 Apr 2018 03:43:23 PM UTC, comment #122: 

@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.


Rik <rik5>
Group administrator
Thu 12 Apr 2018 02:34:13 PM UTC, comment #121: 

@Pantxo,

I'm in a rush.


>> annotation ("textbox", [0 0.5 0.2 0.2], "string",
           "The box should have the same size as the text")
>> __octave_config_info__ ("QT_OFFSCREEN")
ans = 1

The text box is not the same size as the text (about 2x longer).

I'll try the patch later as time permits.

Ben Abbott <bpabbott>
Group Member
Thu 12 Apr 2018 09:42:01 AM UTC, comment #120: 

@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)

Pantxo Diribarne <pantxo>
Group Member
Thu 12 Apr 2018 03:07:13 AM UTC, comment #119: 

hmmm, this is the 2nd time the pdf-file didn't attach :-(

Trying again

Ben Abbott <bpabbott>
Group Member
Thu 12 Apr 2018 03:04:44 AM UTC, comment #118: 

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

Ben Abbott <bpabbott>
Group Member
Wed 11 Apr 2018 02:56:20 PM UTC, comment #117: 

@Ben: Thanks. I think you did not attach the pdf figure.
As for the demos hanging Octave, if you have a version of Qt > 5.1 it should work without osmesa, otherwise Octave should return an error stating that printing invisible figures is not supported.
Does this simple example work?


figure ("visible", "off")
plot (1:10)
print bar.pdf


Pantxo Diribarne <pantxo>
Group Member
Wed 11 Apr 2018 02:38:40 PM UTC, comment #116: 

@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 Abbott <bpabbott>
Group Member
Wed 11 Apr 2018 02:19:36 PM UTC, comment #115: 

@sschoeps: As suggested previously by jpswensen, I attached a patch in which I added debug outputs. Moving the figure from a high to normal dpi (and vice versa) screen should print somthing like "dpr is now 1" then "dpr is now 2". Can you test it?

(file #43880)

Pantxo Diribarne <pantxo>
Group Member
Wed 11 Apr 2018 08:38:45 AM UTC, comment #114: 

@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)

Pantxo Diribarne <pantxo>
Group Member
Wed 11 Apr 2018 04:22:40 AM UTC, comment #113: 

@Pantxo

Your patch (file#43871) works for me. I don't have multiple monitors, so I'm not able to test moving a figure back and forth.

Ben Abbott <bpabbott>
Group Member
Tue 10 Apr 2018 01:45:36 PM UTC, comment #112: 

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)

Pantxo Diribarne <pantxo>
Group Member
Fri 06 Apr 2018 03:04:03 AM UTC, comment #111: 

Ok. I've removed the portion belonging to bug #50025

(file #43818)

Ben Abbott <bpabbott>
Group Member
Thu 05 Apr 2018 04:26:40 PM UTC, comment #110: 

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

Sebastian <sschoeps>
Thu 05 Apr 2018 04:24:18 PM UTC, comment #109: 

Ben Abbot, if I remember correctly, sschoeps had problems with my original per-figure scaling patch. I never had time to iterate back and forth and see why it seemed to work flawlessly on my machine, but not on his. I think I concluded that both of my screens were HiDPI, but of different resolutions.

I also this there is still a problem Linux/Windows handle the Window scaling differently when transferred between HiDPI and non-HiDPI screens because those platforms handle the QT window sizing differently than MacOS.

John Swensen <jpswensen>
Thu 05 Apr 2018 02:57:32 AM UTC, comment #108: 

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

Ben Abbott <bpabbott>
Group Member
Thu 05 Apr 2018 02:56:47 AM UTC, comment #107: 

I've made an attempt for update/rebase the patch. It is attached.

I tried "demo profit". The result looks ok for me.

(file #43808)

Ben Abbott <bpabbott>
Group Member
Tue 03 Apr 2018 11:16:23 PM UTC, comment #106: 

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.

Rik <rik5>
Group administrator
Tue 03 Apr 2018 10:54:10 PM UTC, comment #105: 

The patch no longer applies for me, and I haven’t had the time to fix it

Ben Abbott <bpabbott>
Group Member
Tue 03 Apr 2018 08:47:20 PM UTC, comment #104: 

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

Sebastian

Sebastian <sschoeps>
Tue 20 Feb 2018 06:45:47 PM UTC, comment #103: 

@John:

>> I don't think there is a different opengl_renderer object for each figure, right?


No (or yes :-), a new renderer object is instantiated on the heap each time a figure has to be (re)drawn (see e.g. http://octave.org/doxygen/4.2/de/d86/GLCanvas_8cc_source.html#l00056). Something like:


opengl_renderer r;
r.set_viewport (...)
r.draw (go);


Since opengl_renderer has no static variables, it starts from fresh each time a redraw is requested. So the approach I proposed (and that you already tried) has no reason to not work:


opengl_renderer r;
r.set_viewport (...);
double val = go.get ("__devicePixelRatio__").double_value ();
r.set_devicePixelRatio (val);
r.draw (go);



Pantxo Diribarne <pantxo>
Group Member
Tue 20 Feb 2018 05:15:59 PM UTC, comment #102: 

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 Swensen <jpswensen>
Tue 20 Feb 2018 04:30:02 PM UTC, comment #101: 

I like the suggestion.  Fetching properties through the schema interface isn't truly high performance, whereas grabbing a data member of a class would be very fast.

--Rik

Rik <rik5>
Group administrator
Tue 20 Feb 2018 02:33:07 PM UTC, comment #100: 

@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?

Pantxo Diribarne <pantxo>
Group Member
Sun 11 Feb 2018 03:54:08 AM UTC, comment #99: 

I'm attaching a version of file #43077 that compiles but hangs when I attempt to plot.

(file #43239)

Ben Abbott <bpabbott>
Group Member
Sun 11 Feb 2018 02:49:57 AM UTC, comment #98: 

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


Ben Abbott <bpabbott>
Group Member
Thu 01 Feb 2018 05:43:58 PM UTC, comment #97: 

@John: Excellent.  Also, if the review came across as gruff--excuse that.  I think this is a very important feature to have for 3.4.0 as the entire computing world shifts to HiDPI screens.

Rik <rik5>
Group administrator
Thu 01 Feb 2018 05:41:22 PM UTC, comment #96: 

@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.

John Swensen <jpswensen>
Thu 01 Feb 2018 01:53:48 AM UTC, comment #95: 

I tried the latest patch with Linux and it appears to mostly work.   When shifting a figure from the HiDPI display to a normal one the axes containing the plot is shrunk, however the figure itself is not, so that when you do any operation the axes is autoscaled to match the figure size which is large.

That might be something to do with the version of Linux I am running which is rather old (Mint 18.1 based on Ubuntu 2016.04).

However, the patch has gone backwards from a coding perspective.  There are numerous tabs, the alignments that I contributed in comment #86 are gone, and there are many cuddled parentheses for function calls which is not the Octave convention.

This is a major new feature so there should be an announcement about it added to the NEWS file.  It should please a lot of people.

There is still a problem with the calculated "Extent" property for text objects.  Somewhere in the code there needs to be a division because the calculated value is off by about the scale factor I use for the HiDPI display.

Is there any performance issue associated with having the DPI on a per-figure basis?  Is the value cached somewhere so that it only needs to retrieved when the figure is created or changes screens?  Caching within a property value of the figure isn't necessarily that fast.  Property class objects are huge and unwieldly.  As one example,


+    virtual double devicePixelRatio (const graphics_object* go)
+    {
+      graphics_object fig = go->get_ancestor ("figure");
+      const figure::properties& fp =
+        dynamic_cast<const figure::properties&> (fig.get_properties ());
+      double devicePixelRatio = fp.get___device_pixel_ratio__ ();
+      return devicePixelRatio;
+    }


Rather than get all the properties, if you have a valid graphics_object it might be simpler to just use get()


+    virtual double devicePixelRatio (const graphics_object* go)
+    {
+      graphics_object fig = go->get_ancestor ("figure");
+      return fig.get ("__device_pixel_ratio__").double_value ();
+    }


That same coding strategy can be used in multiple places, but should probably be benchmarked just to make sure it isn't slower.

I think you can avoid work when shifting figures between screens and there is no DevicePixelRatio support.  For example,


+  Figure::figureWindowShown ()
+  {
+    figure::properties& fp = properties<figure> ();
+    QMainWindow *win = qWidget<QMainWindow> ();
+#if defined (HAVE_QSCREEN_DEVICEPIXELRATIO)
+    QWindow* window = win->windowHandle ();
+    QScreen* screen = window->screen ();
+    fp.set___device_pixel_ratio__ (screen->devicePixelRatio ());
+
+    connect (window, SIGNAL (screenChanged (QScreen*)),
+         this, SLOT (screenChanged (QScreen*)));
+#endif
+
+  }


If HAVE_QSCREEN_DEVICEPIXELRATIO is not defined, is there any reason to even get the properties and window?  Why not put the entire function body inside the #if block?


Rik <rik5>
Group administrator
Sat 27 Jan 2018 12:56:16 PM UTC, comment #94: 

@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.

Sebastian <sschoeps>
Fri 26 Jan 2018 03:18:31 PM UTC, comment #93: 

@scchoeps That is strange. It clearly detected the screen change from the HiDPI to non-HiDPI screen (as evidenced by the change in drawing scale), but it didn't seem to detect the change from the non-HiDPI screen back to the HiDPI screen. The exact same screenChanged signal gets fired and the exact same code should handle both of those transitions identically.

I may put some debugging text output back in there and ask you to try again, if that is OK.

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

John Swensen <jpswensen>
Fri 26 Jan 2018 08:35:16 AM UTC, comment #92: 

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)

Sebastian <sschoeps>
Thu 25 Jan 2018 07:04:19 PM UTC, comment #91: 

Attached is an update patch that stores a devicePixelRatio for each individual figure window, detects when the figure is moved between screens and updates the devicePixelRatio accordingly.

Please test and let me know if you have any issues. If you don't have two different screens with different LoDPI and HiDPI, then the behavior of this patch should be no different than the last changeset.

Again, I tried to catch all the coding issues and fix them, but please let me know if I need to fix anything.



(file #43076, file #43077)

John Swensen <jpswensen>
Thu 25 Jan 2018 04:10:29 AM UTC, comment #90: 

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.

John Swensen <jpswensen>
Wed 24 Jan 2018 11:33:44 PM UTC, comment #89: 

I also just confirmed that when switching between screens with a devicePixelRatio of 1.0 and 2.0, the drawing is messed up (being to big or too small depending on which screen you first initialize the QTHandles from).

So, I am proposing one of two things:

1) Each time the screen scale is needed, we walk up the handle tree and determine the screen for the window backing the current operation and query the devicePixelScale

or

2) Store the devicePixelScale in each figure object (query at creation and listen for the QWindow::screenChanged(Screen*) signal after that) and then have anything that needs the screen scale to walk back up the handle tree to its parent figure to retrieve the value.


Thoughts on the two approaches?

John Swensen <jpswensen>
Wed 24 Jan 2018 11:10:21 PM UTC, comment #88: 

@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.

John Swensen <jpswensen>
Wed 24 Jan 2018 09:18:32 PM UTC, comment #87: 

@John: Are you sure that the LineWidth needs to be scaled?  It appears to me that, except for the factor of 4/3 related to the difference between points and pixels, the sizes are already correct.  It could be that OpenGL is already internally scaling things based on resolution so that 1 theoretical pixel is already expanded to the correct number of physical pixels.

Rik <rik5>
Group administrator
Wed 24 Jan 2018 09:14:05 PM UTC, comment #86: 

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)

Rik <rik5>
Group administrator
Wed 24 Jan 2018 08:40:49 PM UTC, comment #85: 

I think the line width adjustment bug has two parts and one of those parts should be part of this patch.

1) Is the line width scaled with HiDPI screens?
2) Is the line width sufficient, after HiDPI issues are fixed?

I think we should definitely scale the glLineWidth based on the HiDPI scale factor as part of this changeset, as it is already fixing many other OpenGL drawing issues that involve HiDPI, but also have a different bug that discusses the correct value for the linewidth.

This fix involves changing line in opengl_renderer::set_linewidth (float w)

glLineWidth (w);

to

glLineWidth (w * display_info::screen_scale ());


As a side note, I am doing some experiments where I am moving windows back and forth between a HiDPI screen and a non-HiDPI screen to figure out how to make that scale factor change based on the window location (for QT only)

John Swensen <jpswensen>
Wed 24 Jan 2018 04:35:26 PM UTC, comment #84: 

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

Rik <rik5>
Group administrator
Wed 24 Jan 2018 04:20:39 PM UTC, comment #83: 

@RIk, I'm crazy busy with the new job. I'll get back to this when the week end rolls around.

Ben Abbott <bpabbott>
Group Member
Wed 24 Jan 2018 04:13:14 PM UTC, comment #82: 

@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%.

Rik <rik5>
Group administrator
Wed 24 Jan 2018 05:21:43 AM UTC, comment #81: 

@Rik: I've added figures with 1pt and 3pt linewidths. My impression is that the widths are scaled ... similar to the images.


Ben Abbott <bpabbott>
Group Member
Wed 24 Jan 2018 03:45:40 AM UTC, comment #80: 

@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.

Rik <rik5>
Group administrator
Wed 24 Jan 2018 03:42:36 AM UTC, comment #79: 

I would make FLTK a secondary concern.  I added this note to the NEWS file a few weeks ago


 ** The FLTK toolkit is no longer prioritized for development.  The
    number of Octave Maintainers is too small to support three different
    graphic toolkits.  New development will target the Qt toolkit.
    While no longer prioritized, the FLTK toolkit is not deprecated and
    there is no schedule for its removal.


This is an important piece of code to have for 3.4 because more and more users will have HiDPI displays.

Rik <rik5>
Group administrator
Sun 21 Jan 2018 12:35:09 AM UTC, comment #78: 

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

Ben Abbott <bpabbott>
Group Member
Sat 20 Jan 2018 08:54:43 PM UTC, comment #77: 

I did a quick comparison of the linewdths renders by Matlab and file #42982. The widths for Octave are about half the width of that in Matlab.

I don't think there is an immediate need to fix this (or that possibilities of multiple monitors) for the initial changeset, but wanted to mention it so that is isn't forgotten. Once a patch is applied we can open new bug reports and close this one.


Ben Abbott <bpabbott>
Group Member
Sat 20 Jan 2018 05:09:27 AM UTC, comment #76: 

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)

Ben Abbott <bpabbott>
Group Member
Sat 20 Jan 2018 04:28:32 AM UTC, comment #75: 

Ben - sorry, the lines of code inside the #if should not be indented further, they should only lead with four spaces, as if the #if weren't there (I think I mentioned that earlier).

What do you mean by detecting HiDPI at run time? Isn't that what this code does when the qt toolkit is initialized? Do you mean if the display scaling factor changes while Octave is running?

John - agree about taking into account the number of people affected. This may also affect Windows users with HiDPI displays. The Windows build of Octave supports all three toolkits. It is currently using FLTK 1.3.4.

Mike Miller <mtmiller>
Group Member
Sat 20 Jan 2018 02:32:42 AM UTC, comment #74: 

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

Ben Abbott <bpabbott>
Group Member
Sat 20 Jan 2018 02:31:56 AM UTC, comment #73: 

Ok. I've taken care of Mike's suggestions. If I've missed something, let me know.

(file #42981)

Ben Abbott <bpabbott>
Group Member
Sat 20 Jan 2018 12:12:53 AM UTC, comment #72: 

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.

John Swensen <jpswensen>
Sat 20 Jan 2018 12:01:03 AM UTC, comment #71: 

Oh, can you please change "QApplication::screens ().at (0)" to "QApplication::primaryScreen ()"? That should be identical but the objective is much clearer.

Mike Miller <mtmiller>
Group Member
Fri 19 Jan 2018 11:50:06 PM UTC, comment #70: 

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.

Mike Miller <mtmiller>
Group Member
Fri 19 Jan 2018 07:42:16 PM UTC, comment #69: 

John's proposal in comment #68 sounds reasonable to me. But I'd ignore gnuplot for now. The gnuplot window can be resized manually and there is no way for Octave to detect the change.

I'd also be ok with ignoring HiDPI on FLTK for the moment. We may end up dropping support for FLTK with HiDPI finds its way into the distributions. If not this decision would encourage FLTK be dropped sooner than later.

Ben Abbott <bpabbott>
Group Member
Fri 19 Jan 2018 07:11:34 PM UTC, comment #68: 

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?

John Swensen <jpswensen>
Fri 19 Jan 2018 06:58:02 PM UTC, comment #67: 

I rather thought about resetting the scree_scale to 1 if graphicsbackends is called with the option fltk. However, you have deeper insight.

Sebastian <sschoeps>
Fri 19 Jan 2018 06:40:17 PM UTC, comment #66: 

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.

John Swensen <jpswensen>
Fri 19 Jan 2018 06:35:56 PM UTC, comment #65: 

I'd say we focus with this patch on fixing qt and I would rather not interfere with FLTK. This is for another time .... The question is anyhow, if Octave should support all 3 backends in the future (there was a recent discussion on the mailing list)

Sebastian <sschoeps>
Fri 19 Jan 2018 06:31:58 PM UTC, comment #64: 

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.

Ben Abbott <bpabbott>
Group Member
Fri 19 Jan 2018 06:25:28 PM UTC, comment #63: 

So does this mean that we now need a check for just Linux (and maybe Windows) such that the window size is scales by the screen_scale. This also begs the question as to how to get this screen scale value when QT is unavailable (not compiled in, but FLTK is).

I need to compile a clean Octave without these new patches and see if the OpenGL scaling is messed up on the mac for FLTK. I assume it is, and this indicated we need to deal with Mac and Linux separately.

John Swensen <jpswensen>
Fri 19 Jan 2018 06:13:36 PM UTC, comment #62: 

@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

Michael Weitzel <mweitzel>
Fri 19 Jan 2018 06:12:35 PM UTC, comment #61: 

@jpswensen: the "error: unexpected exception while parsing input" problem is unrelated to this patch. I just compiled octave (plain default) and it's still happening. However, it does not appear if I call "octave-cli" from the command line which disables the QT graphics completely, while -no-gui is only disabling the drawing of the GUI.

Sebastian <sschoeps>
Fri 19 Jan 2018 06:01:45 PM UTC, comment #60: 

@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

John Swensen <jpswensen>
Fri 19 Jan 2018 05:54:12 PM UTC, comment #59: 

@mweitzel: which version of FLTK are you using? With FLTK 1.3.4 I have proper scaling on macOS with HiDPI.

However, when calling the FLTK backend from the GUI I get a "error: unexpected exception while parsing input". I am currently checking if this is a side-effect from the patch or an unrelated issue of my system.

Sebastian <sschoeps>
Fri 19 Jan 2018 05:52:37 PM UTC, comment #58: 

@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.

John Swensen <jpswensen>
Fri 19 Jan 2018 05:41:44 PM UTC, comment #57: 

@jpswensen switching to FLTK immediately after startup results in a correct figure (though tiny)

Michael Weitzel <mweitzel>
Fri 19 Jan 2018 05:34:20 PM UTC, comment #56: 

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

John Swensen <jpswensen>
Fri 19 Jan 2018 05:24:33 PM UTC, comment #55: 

I forgot to add that I corrected my X-server's DPI setting by creating the file /etc/X11/xorg.conf.d/90-monitor.conf:

Section "Monitor"
Identifier "eDP-1" # as reported by "xrandr -q"
DisplaySize 346 192 # in mm
EndSection

now, the X-sever reports the correct DPI value

[/etc/X11/xorg.conf.d]$ xdpyinfo | grep dots
  resolution:    282x286 dots per inch

Michael Weitzel <mweitzel>
Fri 19 Jan 2018 05:11:00 PM UTC, comment #54: 

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 ...


Michael Weitzel <mweitzel>
Fri 19 Jan 2018 03:09:07 PM UTC, comment #53: 

Oops, I forgot to remove the "retna" reference. New changeset is attached.



(file #42973)

Ben Abbott <bpabbott>
Group Member
Fri 19 Jan 2018 07:03:36 AM UTC, comment #52: 

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

Sebastian <sschoeps>
Fri 19 Jan 2018 03:05:15 AM UTC, comment #51: 

opps, "I think the attached changeset includes all suggested changes."

I've verified the changeset on macOS with Qt5 works as intended.

Ben Abbott <bpabbott>
Group Member
Fri 19 Jan 2018 01:57:23 AM UTC, comment #50: 

@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)

Ben Abbott <bpabbott>
Group Member
Fri 19 Jan 2018 12:33:05 AM UTC, comment #49: 

... and attached for real this time.

(file #42969)

Mike Miller <mtmiller>
Group Member
Fri 19 Jan 2018 12:32:39 AM UTC, comment #48: 

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?

Mike Miller <mtmiller>
Group Member
Thu 18 Jan 2018 10:36:29 PM UTC, comment #47: 

In this case, I think that the HAVE_QT5 is probably sufficient, unless you want to go to the work of making a new configure test. For the time being, it seems that having QT5 will let us know whether to query for a different pixel ratio.

John Swensen <jpswensen>
Thu 18 Jan 2018 09:56:53 PM UTC, comment #46: 

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.

Mike Miller <mtmiller>
Group Member
Thu 18 Jan 2018 09:18:16 PM UTC, comment #45: 

Another patch update to try and address all the coding standard rigmarole.

BTW, the 80 character limit is violated all over the place in the code (that I didn't write) and I personally hate that arbitrary limit dictated by 30 year old hardware. Especially in C++ where something like "octave::math::round" takes up 25% of that and indentation I have no control over takes up as much as another 10% of the 80 character. This leaves coders with the option of making less-than-descriptive variable names or spreading across 8 lines what could have been fit on a single line. </end_rant>

Let me know if you see any other issues.

(file #42967)

John Swensen <jpswensen>
Thu 18 Jan 2018 08:31:19 PM UTC, comment #44: 

@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).


John Swensen <jpswensen>
Thu 18 Jan 2018 08:09:41 PM UTC, comment #43: 

A couple more:

  • the code "1.0 / something" is much clearer than "1./something"
  • use "octave::math::round" instead of "roundf"
Mike Miller <mtmiller>
Group Member
Thu 18 Jan 2018 08:08:14 PM UTC, comment #42: 

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.

Markus Mützel <mmuetzel>
Group administrator
Thu 18 Jan 2018 08:01:48 PM UTC, comment #41: 

Do you want someone to check that Octave builds with the patch, or do you also need someone with a HiDPI display to test the appearance on a GNU/Linux system with Qt 4 or 5?

Some comments on the patch:

  • break lines longer than 80 columns
  • needs more spaces, e.g. between function name and parens, around all '*' operators
  • "QApplication.h" should be "QApplication"
  • we try to avoid version checks completely in Octave, if possible there should be a configure feature test for some function/class/method that is required for the conditional parts of this patch to work. Is devicePixelRatio the function that is only present in Qt 5? Is it the entire class QScreen? Those should be tested for in configure to produce a proper conditional.
Mike Miller <mtmiller>
Group Member
Thu 18 Jan 2018 07:50:27 PM UTC, comment #40: 

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.

Markus Mützel <mmuetzel>
Group administrator
Thu 18 Jan 2018 07:38:08 PM UTC, comment #39: 

Newwest patch built fine on Fedora 27 with Qt4.

Dmitri.

Dmitri A. Sergatskov <dasergatskov>
Thu 18 Jan 2018 07:26:31 PM UTC, comment #38: 

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)

John Swensen <jpswensen>
Thu 18 Jan 2018 06:48:49 PM UTC, comment #37: 

@sschoeps Looks to be. I asked mweitzel to try and build with the latest patch.

John Swensen <jpswensen>
Thu 18 Jan 2018 06:36:14 PM UTC, comment #36: 

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

Sebastian <sschoeps>
Thu 18 Jan 2018 06:34:03 PM UTC, comment #35: 

I don't think a HiDPI monitor is important. I don't even know that this problem will rear its ugly head on any machine other than a Mac because of how they conflate DPI and screen scaling. I just wanted to make sure the proposed patch doesn't cause problems for the QT4 folks on Linux or the QT5 on Linux and Windows.

I will also go back and fix those casts and update the patch.

John Swensen <jpswensen>
Thu 18 Jan 2018 06:32:02 PM UTC, comment #34: 

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?

Markus Mützel <mmuetzel>
Group administrator
Thu 18 Jan 2018 06:31:05 PM UTC, comment #33: 

Fedora 27 has both Qr5 and Qt4.
I built the latest hg tip with you patatch and with
./configure --with-qt=4

There were some warnings:


  CXX      libinterp/corefcn/libinterp_corefcn_libcorefcn_la-inv.lo
  CXX      libinterp/corefcn/libinterp_corefcn_libcorefcn_la-interpreter-private.lo
  CXX      libinterp/corefcn/libinterp_corefcn_libcorefcn_la-interpreter.lo
../libinterp/corefcn/gl-render.cc: In member function ‘virtual void octave::opengl_renderer::setup_opengl_transformation(const axes::properties&)’:
../libinterp/corefcn/gl-render.cc:1162:56: warning: use of old-style cast [-Wold-style-cast]
     glOrtho (0, roundf(screen_scale_inverse*(float)vw[2]), roundf(screen_scale_inverse*(float)vw[3]), 0, xZ1, xZ2);
                                                        ^
../libinterp/corefcn/gl-render.cc:1162:99: warning: use of old-style cast [-Wold-style-cast]
     glOrtho (0, roundf(screen_scale_inverse*(float)vw[2]), roundf(screen_scale_inverse*(float)vw[3]), 0, xZ1, xZ2);
                                                                                                   ^
../libinterp/corefcn/gl-render.cc: In member function ‘virtual void octave::opengl_renderer::draw_text_background(const text::properties&, bool)’:
../libinterp/corefcn/gl-render.cc:3547:56: warning: use of old-style cast [-Wold-style-cast]
     glOrtho (0, roundf(screen_scale_inverse*(float)vp[2]), roundf(screen_scale_inverse*(float)vp[3]), 0, xZ1, xZ2);
                                                        ^
../libinterp/corefcn/gl-render.cc:3547:99: warning: use of old-style cast [-Wold-style-cast]
     glOrtho (0, roundf(screen_scale_inverse*(float)vp[2]), roundf(screen_scale_inverse*(float)vp[3]), 0, xZ1, xZ2);
                                                                                                   ^
../libinterp/corefcn/gl-render.cc: In member function ‘virtual void octave::opengl_renderer::init_marker(const string&, double, float)’:
../libinterp/corefcn/gl-render.cc:4124:56: warning: use of old-style cast [-Wold-style-cast]
     glOrtho (0, roundf(screen_scale_inverse*(float)vw[2]), roundf(screen_scale_inverse*(float)vw[3]), 0, xZ1, xZ2);
                                                        ^
../libinterp/corefcn/gl-render.cc:4124:99: warning: use of old-style cast [-Wold-style-cast]
     glOrtho (0, roundf(screen_scale_inverse*(float)vw[2]), roundf(screen_scale_inverse*(float)vw[3]), 0, xZ1, xZ2);
                                                                                                   ^


but it seems to run fine.
I can try some tests, but I do not have HDPI monitor.

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Thu 18 Jan 2018 04:58:28 PM UTC, comment #32: 

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.

John Swensen <jpswensen>
Thu 18 Jan 2018 04:00:30 PM UTC, comment #31: 

@Mike/Pantxo,

Would you look over John's patch and comment compliance to Octave's coding style conventions? It looks ok to me, but I don't trust myself with C++.

@John,

Can you put together a ChangeLog describing the changes? Once the coding style are verified, I can construct the changeset in your name and commit.

I'm in a bit of a rush, since I'll be starting a new job on Monday and may not have time to get back to this for a while.

Ben Abbott <bpabbott>
Group Member
Thu 18 Jan 2018 06:35:39 AM UTC, comment #30: 

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

Sebastian <sschoeps>
Thu 18 Jan 2018 02:04:18 AM UTC, comment #29: 

@John, the patch (file #42946) applied cleanly for me and worked nicely with all image, imagesc, and colorbar demos.


Ben Abbott <bpabbott>
Group Member
Thu 18 Jan 2018 01:34:15 AM UTC, comment #28: 

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 Swensen <jpswensen>
Wed 17 Jan 2018 01:13:14 AM UTC, comment #27: 

I'm adding an update to file #42910

(file #42932)

Ben Abbott <bpabbott>
Group Member
Mon 15 Jan 2018 02:39:22 AM UTC, comment #26: 

@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)

Ben Abbott <bpabbott>
Group Member
Mon 15 Jan 2018 02:32:24 AM UTC, comment #25: 

@Ben

Yes, scripts like that would help. I think i had figured out a way to get the Homebrew script to build and install, but not remove the build directory, but now I forget how I did that. A script would be better because then I can isolate where it gets installed and not pollute my Homebrew setup.

John Swensen <jpswensen>
Sun 14 Jan 2018 04:27:57 PM UTC, comment #24: 

@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?

Ben Abbott <bpabbott>
Group Member
Sun 14 Jan 2018 04:25:45 PM UTC, comment #23: 

Regarding whether the scaling problem happens on OSX with Qt4.8, I do not see the scaling problem when using Qt 4.8.7.

For those interested, a Homebrew formula for Qt 4.8.7 is available at the link below.

https://github.com/cartr/homebrew-qt4/blob/master/qt%404.rb

If I understand correctly

1) file #42901 is incompatible with Qt4
2) file #42901 not fix the scaling problem for images.
3) file #42901 needs style fixes to conform to Octave coding standards

Thus, the build a functional Octave Qt4 is necessary. I plan to stick with Qt5.

Ben Abbott <bpabbott>
Group Member
Sat 13 Jan 2018 04:16:42 PM UTC, comment #22: 

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

(file #42901)

Ben Abbott <bpabbott>
Group Member
Sun 18 Dec 2016 02:29:04 PM UTC, comment #21: 

Hi,

I just tested the patch on Qt 5, macOS, and a HiDPI (including the patch added via homebrew/science/octave)

It works fine for plotting points, but does not work for imagesc and the colorbar:

numberPoints = 100;
imagesc(rand(numberPoints));
hold on;
plot(1:numberPoints,numberPoints*rand(1,numberPoints),'.')
colorbar;

A fix is highly appreciated!

Thanks a lot for your efforts!

Regards,
Stefan


Stefan Jaumann <sjaumann>
Thu 17 Nov 2016 08:20:06 PM UTC, comment #20: 

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.

Mike Miller <mtmiller>
Group Member
Thu 17 Nov 2016 05:03:08 PM UTC, comment #19: 

Mike Miller, can you confirm that the scaling problem happens on OSX with Qt4.8? This problem wasn't happening back with 4.0.3 provided by Homebrew, but I wasn't sure if it started happening because of the switch to Qt5 or if there was a change in Octave.

If you are also seeing the scaling issues with Octave 4.2 and Qt4.8, then I can try to come up with a solution that checks the Qt version and gets the screen scaling appropriately.

Do you have a suggested as to where to keep the screen scaling? We could have it as a property in the root graphics object. Does that seem like a better solution? Or, I could make it a non-public static variable and write accessors for it. I am open to what others think is best.

John Swensen <jpswensen>
Thu 17 Nov 2016 08:03:57 AM UTC, comment #18: 

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

Sebastian <sschoeps>
Thu 17 Nov 2016 04:14:43 AM UTC, comment #17: 

This patch needs some style fixes to conform to Octave coding standards. It will also not compile with Qt 4.8 because the devicePixelRatio property was not added until midway through the Qt 5 development series. I also think a public static variable (essentially a program-global value) in the gh_manager class is probably not the best way to store the scaling factor. If someone with Qt 5, macOS, and a HiDPI display wants to test to see if the patch works functionally please do.

Mike Miller <mtmiller>
Group Member
Tue 08 Nov 2016 07:47:08 AM UTC, comment #16: 

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)

John Swensen <jpswensen>
Tue 08 Nov 2016 06:59:43 AM UTC, comment #15: 

OK. I created an updated patch that fixed the problems with the last patch.  I don't know what you all think the best way to keep track of the screen scale. I added it as a static member variable of the gh_manager class.

This should take care of both scaling the plot area for retina displays, ensure single markers are plotted in the correct location, and it scales the axes marker text sizes and the xlabel, ylabel, title, and legend text sizes.

It shouldn't affect a non-retina display because the scale factor will always be 1.0.

(file #38901)

John Swensen <jpswensen>
Mon 07 Nov 2016 07:13:27 PM UTC, comment #14: 

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.


John Swensen <jpswensen>
Mon 31 Oct 2016 03:25:48 PM UTC, comment #13: 

I also observe the gui freeze when exiting. However, I have found that if I do a "close all" before calling exit that it doesn't seem to happen. I think that could probably help narrow down what is causing the freeze.

John Swensen <jpswensen>
Sat 29 Oct 2016 09:51:12 AM UTC, comment #12: 

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?

Sebastian <sschoeps>
Fri 28 Oct 2016 05:46:57 PM UTC, comment #11: 

Here is a patch that is a beginning to fixing the problem. This fixes the problem of the axis size and axis label scaling. I have another patch that fixes the font size scaling for other text object, but the problem is that they are overlapping the axis label. I suspect there are going to be a ton of places where position/size will need to be scaled by the screen scale.

I also don't know the guts of the handle graphics subsystem well enough to have added another property. I suspect we want the screen scale to be a top-level property, rather than a variable in gl_renderer and GLCanvas like I did in this patch.

I can continue to work on this, but was wondering if the the handle graphics system is abstracted enough that there are easy-to-find places where screen_scale should be applied?

(file #38835)

John Swensen <jpswensen>
Thu 27 Oct 2016 06:13:44 PM UTC, comment #10: 

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.

John Swensen <jpswensen>
Fri 21 Oct 2016 08:59:24 PM UTC, comment #9: 
Sebastian <sschoeps>
Sun 09 Oct 2016 07:06:54 PM UTC, comment #8: 

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

Sebastian <sschoeps>
Sun 09 Oct 2016 05:58:04 PM UTC, comment #7: 

Sebastian, I am surprised that people propose here and there to distribute Qt5 builds. Considering the first Octave/Qt5 binaries have come out a few weeks (mounth?) ago on linux and Mac do you think this a sufficient amount of time/testing?
At least this very bug shows that being able to build Octave with Qt5 is not the end of the story.

Pantxo Diribarne <pantxo>
Group Member
Sat 08 Oct 2016 01:26:29 PM UTC, comment #6: 

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.?

Sebastian <sschoeps>
Wed 14 Sep 2016 05:00:07 PM UTC, comment #5: 

Retitled for easier searching. There have also been bugs reported about similar scaling issues on Windows systems with high resolution monitors and HiDPI scaling enabled.

Mike Miller <mtmiller>
Group Member
Wed 14 Sep 2016 03:20:20 PM UTC, comment #4: 

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


Sebastian <sschoeps>
Wed 14 Sep 2016 02:59:04 PM UTC, comment #3: 

Thanks, are you able to test fltk in pure cli mode (launched using "octave -cli" or "octave --no-gui-libs") with this retina display. Does it suffer the same issue?

In Qt5 the way do detect the scale factor seams to be the use of QWindow::devicePixelRatio ().

Pantxo Diribarne <pantxo>
Group Member
Wed 14 Sep 2016 02:41:19 PM UTC, comment #2: 

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




Sebastian <sschoeps>
Tue 13 Sep 2016 09:05:40 PM UTC, comment #1: 

According to this [1] I understand that on mac, even though HDPI resolution is hidden to the developper by the use of device independent pixel units (for widgets and graphics views), pixels in an opengl scene are still screen pixels:


...
OpenGL is a device pixel based API and remains so in high-dpi mode. There is a flag on NSView to enable/disable the 2x scaling – Qt sets it in all cases. Shaders run in device pixels.
...


IIUC, we should detect such high dpi screens on Mac and scale distances (and font size when expressed in pixels) in opengl by a factor of 2.

Sebastian, what does "get (0, 'screenpixelsperinch')" return?


[1] http://blog.qt.io/blog/2013/04/25/retina-display-support-for-mac-os-ios-and-x11/

Pantxo Diribarne <pantxo>
Group Member
Sat 10 Sep 2016 03:51:42 PM UTC, original submission:  

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

Sebastian <sschoeps>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #45065:  btndown_hdpi6.patch added by pantxo (11KiB - text/x-patch)
file #45059:  btndown_hdpi5.patch added by pantxo (10KiB - text/x-patch)
file #45032:  btndown_hdpi3.patch added by pantxo (10KiB - text/x-patch)
file #45031:  bug49053-qt4.diff added by mtmiller (697B - text/x-patch)
file #45030:  build_qt4.patch added by pantxo (803B - text/x-patch)
file #45026:  btndown_hdpi2.patch added by pantxo (8KiB - text/x-patch)
file #45025:  buttondownfcn box.png added by bpabbott (403KiB - image/png)
file #45021:  btndown_hdpi.patch added by pantxo (958B - text/x-patch)
file #45016:  high_res_update8.patch added by pantxo (19KiB - text/x-patch)
file #45012:  hold6.svg added by bpabbott (573KiB - image/svg+xml)
file #45009:  high_res_update7.patch added by pantxo (18KiB - text/x-patch)
file #44985:  high_res_update6.diff added by bpabbott (15KiB - application/octet-stream)
file #44041:  high_res_update5.patch added by jpswensen (14KiB - application/octet-stream)
file #43956:  seb_screenshot_1.png added by sschoeps (102KiB - image/png)
file #43957:  seb_screenshot_2.png added by sschoeps (136KiB - image/png)
file #43958:  seb_screenshot_3.png added by sschoeps (113KiB - image/png)
file #43892:  high_res_update4.patch added by pantxo (14KiB - text/x-patch)
file #43880:  high_res_update3.patch added by pantxo (12KiB - text/x-patch)
file #43879:  dump_plot_demos.m added by pantxo (379KiB - text/x-objcsrc)
file #43871:  high_res_update2.patch added by pantxo (12KiB - text/x-patch)
file #43818:  per_figure_displaypixelratio_2018Apr05.diff added by bpabbott (15KiB - application/octet-stream)
file #43808:  per_figure_displaypixelratio_2018Apr04.diff added by bpabbott (16KiB - application/octet-stream)
file #43239:  per_figure_displaypixelratio_feb09.diff added by bpabbott (15KiB - application/octet-stream)
file #43087:  mac_no_hidpi.mp4 added by sschoeps (2MiB - video/mp4)
file #43076:  per_figure_displaypixelratio.changelog added by jpswensen (2KiB - application/octet-stream)
file #43077:  per_figure_displaypixelratio.diff added by jpswensen (15KiB - application/octet-stream)
file #43070:  hidpi.20180124.cset added by rik5 (11KiB - application/octet-stream)
file #42993:  Matlab linewidth.png added by bpabbott (186KiB - image/png)
file #42994:  Octave linewidth.png added by bpabbott (187KiB - image/png)
file #42982:  changeset.patch added by bpabbott (11KiB - application/octet-stream)
file #42981:  changeset.patch added by bpabbott (11KiB - application/octet-stream)
file #42973:  changeset.patch added by bpabbott (11KiB - application/octet-stream)
file #42970:  changeset.patch added by bpabbott (11KiB - application/octet-stream)
file #42969:  bug-49053-config-test.patch added by mtmiller (2KiB - application/octet-stream)
file #42946:  retina_scale_hg_displayinfo.patch added by jpswensen (7KiB - application/octet-stream)
file #42932:  homebrew_configure_octave_v3.sh added by bpabbott (2KiB - application/x-sh)
file #42910:  homebrew_configure_octave_v2.sh added by bpabbott (2KiB - application/x-sh)
file #42911:  homebrew_make_octave_v2.sh added by bpabbott (247B - application/x-sh)
file #42901:  retina_scale_hg_patch_6ece7c7e4912.diff added by bpabbott (4KiB - application/octet-stream)
file #39268:  octave-scaling.jpeg added by sjaumann (210KiB - image/jpeg - test octave scaling patch)
file #38902:  retina_scale_hg_patch.diff added by jpswensen (4KiB - application/octet-stream)
file #38901:  screen_scale_labels_also.diff added by jpswensen (4KiB - application/octet-stream)
file #38866:  screen_scale_patch420rc3.diff added by sschoeps (4KiB - application/octet-stream - the same as #38835 but for Octave 4.2.0 rc3)
file #38835:  screen_scale_patch.diff added by jpswensen (3KiB - application/octet-stream)
file #38507:  fltk_more_space.png added by sschoeps (228KiB - image/png)
file #38506:  fltk_default.png added by sschoeps (189KiB - image/png)
file #38505:  pref_more_space.png added by sschoeps (260KiB - image/png)
file #38504:  pref_default.png added by sschoeps (186KiB - image/png)
file #38471:  screenshot.png added by sschoeps (168KiB - image/png)

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by henryiii (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by mweitzel (Updated the item)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by dasergatskov (Posted a comment)
  • -email is unavailable- added by bpabbott (Updated the item)
  • -email is unavailable- added by jpswensen (Posted a comment)
  • -email is unavailable- added by pantxo (Posted a comment)
  • -email is unavailable- added by sschoeps (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 25 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-09-21 pantxo StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2018-09-20 pantxo Attached File- Added btndown_hdpi6.patch, #45065
    2018-09-20 pantxo Attached File- Added btndown_hdpi5.patch, #45059
    2018-09-19 mtmiller Carbon-CopyRemoved 80942 -
    2018-09-18 pantxo Attached File- Added btndown_hdpi3.patch, #45032
    2018-09-17 mtmiller Attached File- Added bug49053-qt4.diff, #45031
    2018-09-17 pantxo Attached File- Added build_qt4.patch, #45030
    2018-09-17 pantxo Attached File- Added btndown_hdpi2.patch, #45026
    2018-09-16 bpabbott Attached File- Added buttondownfcn box.png, #45025
    2018-09-16 bpabbott Attached File- Added buttondownfnc figure.png, #45022
    2018-09-16 pantxo Attached File- Added btndown_hdpi.patch, #45021
    2018-09-15 pantxo StatusPatch Submitted Ready For Test
    2018-09-15 pantxo Attached File- Added high_res_update8.patch, #45016
    2018-09-14 bpabbott Attached File- Added hold6.svg, #45012
    2018-09-14 pantxo Attached File- Added high_res_update7.patch, #45009
    2018-09-14 sschoeps Attached File- Added plot_macos_font_issue.png, #45007
    2018-09-12 bpabbott Attached File- Added high_res_update6.diff, #44985
    2018-05-15 kejpi Attached File- Added Screenshot_20180515_205841_Octave_4.4.png, #44162
    2018-04-27 jpswensen Attached File- Added high_res_update5.patch, #44041
    2018-04-17 sschoeps Attached File- Added seb_screenshot_1.png, #43956
        Attached File- Added seb_screenshot_2.png, #43957
        Attached File- Added seb_screenshot_3.png, #43958
    2018-04-13 mmuetzel Dependencies- bugs #49612 is dependent
    2018-04-12 pantxo Attached File- Added high_res_update4.patch, #43892

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code