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

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

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

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.

(file #44162)

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.

(file #43956, file #43957, file #43958)

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

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>
Project 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>
Project Administrator
Thu 12 Apr 2018 02:34:13 PM UTC, comment #121:

@Pantxo,

I'm in a rush.

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

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

(file #43892)

Pantxo Diribarne <pantxo>
Project 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>
Project Member
Thu 12 Apr 2018 03:04:44 AM UTC, comment #118:

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

and

and

The file foo.pdf is attached

Ben Abbott <bpabbott>
Project 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?

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

@Pantxo

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

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:

Pantxo Diribarne <pantxo>
Project 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>
Project 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>
Project 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>
Project 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>
Project 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>
Project 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,

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

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,

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

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

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

(file #43043, file #43044, file #43045, file #43046)

Ben Abbott <bpabbott>
Project 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>
Project 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

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

Rik <rik5>
Project 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>
Project 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.

(file #42993, file #42994)

Ben Abbott <bpabbott>
Project 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>
Project 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>
Project Administrator
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>
Project 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>
Project 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>
Project Administrator
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>
Project Administrator
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>
Project 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>
Project 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 ...

(file #42975, file #42976, file #42977)

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>
Project 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>
Project 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>
Project Member
Fri 19 Jan 2018 12:33:05 AM UTC, comment #49:

... and attached for real this time.

(file #42969)

Mike Miller <mtmiller>
Project Administrator
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>
Project Administrator
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>
Project Administrator
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>
Project Administrator
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>
Project Member
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>
Project Administrator
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:

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

Markus Mützel <mmuetzel>
Project Member
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>
Project Member
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:

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

(file #39268)

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>
Project Administrator
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>
Project Administrator
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:

I just realized that Matlab has the same issue: https://de.mathworks.com/matlabcentral/answers/241982-plots-are-shown-as-upper-left-cropped-on-osx-in-2015b

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>
Project 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>
Project Administrator
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

(file #38506, file #38507)

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

(file #38504, file #38505)

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:

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>
Project 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 #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 kejpi (Updated the item)
  • -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 mtmiller (Posted a comment)
  • -email is unavailable- added by pantxo (Posted a comment)
  • -email is unavailable- added by sschoeps (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 25 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    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
    2018-04-11 pantxo Attached File- => Added high_res_update3.patch, #43880
    2018-04-11 pantxo Attached File- => Added dump_plot_demos.m, #43879
    2018-04-10 pantxo Attached File- => Added high_res_update2.patch, #43871
    2018-04-06 bpabbott Attached File- => Added per_figure_displaypixelratio_2018Apr05.diff, #43818
    2018-04-05 bpabbott Attached File- => Added per_figure_displaypixelratio_2018Apr04.diff, #43808
    2018-02-11 bpabbott Attached File- => Added per_figure_displaypixelratio_feb09.diff, #43239
    2018-01-26 sschoeps Attached File- => Added mac_no_hidpi.mp4, #43087
    2018-01-25 jpswensen Attached File- => Added per_figure_displaypixelratio.changelog, #43076
        Attached File- => Added per_figure_displaypixelratio.diff, #43077
    2018-01-24 rik5 Attached File- => Added hidpi.20180124.cset, #43070
    2018-01-24 rik5 Release4.2.0 => dev
        Operating SystemMac OS => Any
    2018-01-24 bpabbott Attached File- => Added Octave linewidth=1pt.png, #43043
        Attached File- => Added Octave linewidth=3pt.png, #43044
        Attached File- => Added Matlab linewidth=1pt.png, #43045
        Attached File- => Added Matlab linewidth=3pt.png, #43046
    2018-01-20 bpabbott Attached File- => Added Matlab linewidth.png, #42993
        Attached File- => Added Octave linewidth.png, #42994

    Back to the top


    Powered by Savane 3.3