bugGNU Octave - Bugs: bug #48446, uicontrols extent is incorrect

 
 

bug #48446: uicontrols extent is incorrect

Submitted by:  Andreas Weber <andy1978>
Submitted on:  Fri 08 Jul 2016 12:18:03 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

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)

Wed 17 Jul 2019 09:22:17 PM UTC, comment #13: 

Clsoing report after testing further myself.

Pantxo Diribarne <pantxo>
Project Member
Mon 15 Jul 2019 09:47:17 PM UTC, comment #12: 

@Guillaume: Thanks for testing. The remaining issue is bug #45600.

I pushed the patch here:

http://hg.savannah.gnu.org/hgweb/octave/rev/e3d886685813

Marking ready for test.

Pantxo Diribarne <pantxo>
Project Member
Mon 15 Jul 2019 02:12:46 PM UTC, comment #11: 

I tried the patch on the code from comment #3 and I now indeed get larger values for the uicontrol extent compared to the text one. It also improves what I was observing in comment #7: I assume the remaining difference is due to the inconsistency between OpenGL and Qt you are mentioning. Is that another bug report?
Many thanks Pantxo!

Guillaume <gyom>
Thu 11 Jul 2019 08:59:54 PM UTC, comment #10: 

I attached a patch that fixes the issue for me. The height of multi-line strings (like in listbox style uicontrols) is approximated assuming the total height is nlines times the maximum height of individual lines.

This doesn't change the inconsistency between the size of strings rendered in OpenGL and in Qt.

(file #47197)

Pantxo Diribarne <pantxo>
Project Member
Sat 08 Sep 2018 10:58:57 AM UTC, comment #9: 

@gyom: My approach would be to add a virtual method to the BaseControl class (e.g. BaseControl::update_text_extent) and let BaseControl::update call it when the font changes and style specific StyleControl::update methods call it when the string itself changes.

BaseControl::update_text_extent would use QFontMetrics::boundingRect [1] to get the bounding box of the text contained in the "string" property and then set the uicontrol object's "__extent__" property (which you should add in graphics.in.h). If some special treatment is needed, depending on the uicontrol style, then BaseControl::update_text_extent can be overloaded in the style specific class (SliderControl, ToggleButtonControl, ...).

Then you could eliminate uicontrol::properties::update_text_extent() and let uicontrol::properties::get_extent use _extent_ instead of extent.

[1] https://doc.qt.io/qt-5/qfontmetrics.html#boundingRect-1

Pantxo Diribarne <pantxo>
Project Member
Fri 07 Sep 2018 01:34:48 PM UTC, comment #8: 

Hi Pantxo and Andreas,

I'm back to this issue with the extent property of uicontrols. I'm looking at uicontrol::properties::update_text_extent() in libinterp/corefcn/graphics.cc. Would you be able to guide me in how to implement and test the suggestion of comment #5?

Thanks

Guillaume <gyom>
Wed 13 Jul 2016 09:09:47 AM UTC, comment #7: 

I also noticed this issue with uicontrols' extent: would you be able to provide me with a way to test whether the suggestion of comment #5 solves all of the related issues I am observing?

Guillaume <gyom>
Sat 09 Jul 2016 02:24:28 PM UTC, comment #6: 

@Andreas: I totally agree. This will leave us with the current inconsistency between string size in uixx and text objects, but at list the returned extent will be correct.

Pantxo Diribarne <pantxo>
Project Member
Sat 09 Jul 2016 01:01:47 PM UTC, comment #5: 

@Pantxo: Okay, but then the extent property of a uicontrol should IMHO return the value calculated by the used graphics_toolkit, in case of Qt

width = label.fontMetrics().boundingRect(label.text()).width()

And not create a text_renderer to calculate it as it's done in uicontrol::properties::update_text_extent or am I wrong?

Andreas Weber <andy1978>
Project Member
Fri 08 Jul 2016 03:38:35 PM UTC, comment #4: 

It is a known issue that text objects have an underestimated size, see bug #45600.

To sum up: it is easy to have the right font size, simply feed "FT_Set_Char_Size" with the right screen resolution (not 0 which means 72 pixels/inch = point size) in ft_text_renderer::ft_font::get_face (). But this will lead to printing issues in the current state of the printing system.

Pantxo Diribarne <pantxo>
Project Member
Fri 08 Jul 2016 01:18:52 PM UTC, comment #3: 

Okay, the problem is that "normal" text in a plot and text uicontrol have different sizes.

close all
figure

t2 = text (250, 250, "ABC", "fontsize", 50, "units", "pixels");
t1 = uicontrol ("style", "text",
               "fontsize", 50,
               "position", [100 100 200 100],
               "string", "ABC");

get (t1, "extent")(3:4)
get (t2, "extent")(3:4)

produces attached plot and returns [102 58] twice but the uicontrol text is larger.


Andreas Weber <andy1978>
Project Member
Fri 08 Jul 2016 12:59:25 PM UTC, comment #2: 

The bbox calculations in ft-text-renderer.cc seems fine (the advance is caluclated in http://hg.octave.org/octave/file/tip/libinterp/corefcn/ft-text-renderer.cc#l808)

The problem is that the extent property from uicontrol asks ft-text-renderer but the uicontrol is qt object which doesn't use this renderer to display text. I only can imagine that there is a fontsize or -name mismatch.

Andreas Weber <andy1978>
Project Member
Fri 08 Jul 2016 12:26:08 PM UTC, comment #1: 

Hm sorry, looks related to #44473 but I don't see the factor 2 and think this is more detailed.

Andreas Weber <andy1978>
Project Member
Fri 08 Jul 2016 12:18:03 PM UTC, original submission:  

Consider following code:

close all
graphics_toolkit qt
figure
t = uicontrol ("style", "text");
set (t, "string", "ABCDEFGH");
get (t, "extent")
ans =
    1    1   59   11

If I make a screenshot and measure the real width its approx 80 pixels.

As far as I can see the code in ft-text-renderer.cc ft_text_renderer::compute_bbox sums up the glyph width and not the "advance width" (https://www.freetype.org/freetype2/docs/glyphs/glyphs-3.html) so the space between the glyphs is missing from the sum.

Andreas Weber <andy1978>
Project Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #47197:  extent.patch added by pantxo (6KiB - text/x-patch)
file #37768:  uicontrol_extent_1.png added by andy1978 (10KiB - image/png)

 

Carbon-Copy List
  • -email is unavailable- added by gyom (Posted a comment)
  • -email is unavailable- added by pantxo (Posted a comment)
  • -email is unavailable- added by andy1978 (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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-07-17 pantxo StatusReady For Test => Fixed
        Open/ClosedOpen => Closed
    2019-07-15 pantxo StatusPatch Submitted => Ready For Test
    2019-07-14 pantxo Dependencies- => bugs #56146 is dependent
    2019-07-11 pantxo Attached File- => Added extent.patch, #47197
        StatusNone => Patch Submitted
    2016-07-08 mtmiller Dependencies- => bugs #44473 is dependent
    2016-07-08 andy1978 Attached File- => Added uicontrol_extent_1.png, #37768
        Summaryuicontrol extent returns sum of glyph width instead sum of advance width => uicontrols extent is incorrect

    Back to the top


    Powered by Savane 3.5