After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 721627 - bounds of text items in goocanvasmm are wrong
bounds of text items in goocanvasmm are wrong
Status: RESOLVED FIXED
Product: goocanvasmm
Classification: Other
Component: general
1.90.x
Other Linux
: Normal normal
: ---
Assigned To: goocanvasmm-maint
goocanvasmm-maint
Depends on:
Blocks:
 
 
Reported: 2014-01-06 11:49 UTC by Klaus Rudolph
Modified: 2014-01-12 15:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Example shows bug for text bounds in goocanvasmm (1.02 KB, text/x-c++src)
2014-01-06 12:23 UTC, Klaus Rudolph
  Details
patch: ItemSimple: Fix get_bounds() (1.28 KB, patch)
2014-01-07 15:00 UTC, Kjell Ahlstedt
none Details | Review
patch: Remove ItemSimple::get_bounds() (1.13 KB, patch)
2014-01-07 18:09 UTC, Kjell Ahlstedt
none Details | Review

Description Klaus Rudolph 2014-01-06 11:49:32 UTC
I try to get the height and width of a rendered text in a goocanvasmm. But all I tried results in 0 values for all parameters in Bounds object.

If I insert a c-function call to the txt item I get the correct values in the c++ interface! This looks very strange to me!

See the following code lines



Glib::RefPtr<Goocanvas::Text> txt = Goocanvas::Text::create( "W123", 0, 0 );
Goocanvas::Bounds b = txt->get_bounds();

// If the following 2 lines are removed, the output for all values are 0!
// if the two lines are inserted, the values for 'b' are valid.
GooCanvasBounds bounds;
goo_canvas_item_get_bounds (( GooCanvasItem*)txt->gobj(), &bounds);


cout << b.get_x1() << endl;
cout << b.get_x2() << endl;
cout << b.get_y1() << endl;
cout << b.get_y2() << endl;
Comment 1 Murray Cumming 2014-01-06 12:00:02 UTC
Thanks for the bug report. Could you please provide a small-as-possible test case that shows this? And, maybe even try if the same thing happens when using just the C API?
Comment 2 Klaus Rudolph 2014-01-06 12:23:10 UTC
Created attachment 265429 [details]
Example shows bug for text bounds in goocanvasmm 

This file provides a full example to reproduce the described bug.
Comment 3 Kjell Ahlstedt 2014-01-07 15:00:27 UTC
Created attachment 265537 [details] [review]
patch: ItemSimple: Fix get_bounds()

Class hierarchy:
  class Goocanvas::Item       : public Glib::Interface
  class Goocanvas::ItemSimple : public Glib::Object, public Goocanvas::Item
  class Goocanvas::Text       : public Goocanvas::ItemSimple

Goocanvas::ItemSimple::get_bounds() overrides Goocanvas::Item::get_bounds().
It shouldn't do that. Goocanvas::ItemSimple::get_bounds() should be removed.
Until we can break ABI, let it call Goocanvas::Item::get_bounds().

Goocanvas::ItemSimple::get_bounds() copies directly from
GooCanvasItemSimple::bounds without making sure that it's updated.
Goocanvas::Item::get_bounds() calls goo_canvas_item_get_bounds(), which calls
goo_canvas_item_simple_get_bounds(), which updates the bounds if necessary.

I also wonder if there shall really be a Goocanvas::ItemSimple::set_bounds()
method? Or, if it's useful, shouldn't it be protected? I'm not very familiar
with goocanvas[mm]. I can be wrong, but I get the impression that the bounds
shall be calculated by the GooCanvasItemSimple object itself.
Comment 4 Murray Cumming 2014-01-07 15:27:53 UTC
Thanks.

(In reply to comment #3)
> Goocanvas::ItemSimple::get_bounds() overrides Goocanvas::Item::get_bounds().
> It shouldn't do that. Goocanvas::ItemSimple::get_bounds() should be removed.
> Until we can break ABI, let it call Goocanvas::Item::get_bounds().

If it could never be used successfully without this odd workaround, maybe we can safely just remove it. The API and ABI are not officially stable yet, and I wonder if this would cause any real disruption to anyone at all.
Comment 5 Kjell Ahlstedt 2014-01-07 18:09:26 UTC
Created attachment 265555 [details] [review]
patch: Remove ItemSimple::get_bounds()

How about this patch then?

If you know that GooCanvasItemSimple.bounds is up to date, you could safely
use ItemSimple::get_bounds() and save a fraction of a microsecond.
IMO the very small saving of time is not worth the complication it would
be for the application programmer to choose between the faster
ItemSimple::get_bounds() and the safer Item::get_bounds().
Comment 6 Kjell Ahlstedt 2014-01-12 15:49:25 UTC
Since no one has criticized my patch, I have pushed it.