GNOME Bugzilla – Bug 332266
gdk_draw_layout fails for coordinates >= 2^21
Last modified: 2011-02-04 16:10:53 UTC
Please describe the problem: gdk_draw_layout takes the gint pixel coordinates and multiplies by PANGO_SCALE (2^10) before passing to pango_renderer_draw_layout. Steps to reproduce: Attempting to use gnumeric with > 134,000 rows generates y coordinates larger than 2^21 (default is 17 pixels/row at 100% zoom). Actual results: Expected results: Does this happen every time? Other information:
We'll likely need some pango api extension to handle this.
Guess this can be easily solved in gdkpango.c already. There is code like this: /* When we have a matrix, we do positioning by adjusting the matrix, and * clamp just pass x=0, y=0 to the lower levels. We don't want to introduce * a matrix when the caller didn't provide one, however, since that adds * lots of floating point arithmetic for each glyph. */ matrix = pango_context_get_matrix (pango_layout_get_context (layout)); if (matrix) { PangoMatrix tmp_matrix; GdkRectangle rect; get_rotated_layout_bounds (layout, &rect); tmp_matrix = *matrix; tmp_matrix.x0 += x - rect.x; tmp_matrix.y0 += y - rect.y; pango_renderer_set_matrix (renderer, &tmp_matrix); x = 0; y = 0; } else pango_renderer_set_matrix (renderer, NULL); It just needs to use a matrix if (x,y) are out of range too... The range will be something like PANGO_PIXELS (G_MAXINT)...
While around there, seems to me like the same code in gdk_draw_layout_line_with_colors can use this patch: if (matrix) { PangoMatrix tmp_matrix; tmp_matrix = *matrix; - tmp_matrix.x0 = x; - tmp_matrix.y0 = y; + tmp_matrix.x0 += x; + tmp_matrix.y0 += y; pango_renderer_set_matrix (renderer, &tmp_matrix); x = 0; y = 0; }
behad : Your suggestion works, but the performance significantly degraded. I supposed anyone foolish enough to start scrolling around at rows > 134,000 deserves what they get (let's not discuss the wisdom of using a spreadsheet for that magnitude of data) but it's not an ideal solution. At a minimum the gdk interface should document that coords must be < 2^21.
Created attachment 59980 [details] [review] implement behad's suggested kludge. It's not pretty but it gets the job done.
Jody: Is it with cairo backend? I think (but not sure) that with cairo, using a matrix or not doesn't make any difference performance-wise. A similar patch should be applied to gdk_draw_layout_line_with_colors too...
Behdad: compare http://bugzilla.gnome.org/show_bug.cgi?id=316597#c4
so, what we need is a) a complete patch that covers all functions where we use pango scaling b) as jody says, some documentation on the acceptable range of coordinates
It's more like one or the other. With a), there's no limit on coordinates anymore, at the cost of paying for matrix operations, so we may want to document that, but I don't see it essential at all.
Created attachment 61790 [details] [review] Use a matrix when scaling would overflow
We could go so far as to export that utility function from pango instead of copying it all over the place.
Makes sense. If we want to be complete, we should add the following six functions: pango_layout_get_rotated_size pango_layout_get_rotated_pixel_size pango_layout_get_rotated_extents pango_layout_get_rotated_pixel_extents pango_layout_line_get_rotated_extents pango_layout_line_get_rotated_pixel_extents Any thoughts, Owen? Like for example, the _size ones are not after all a good idea?
since the _size functions are just trivial wrappers around the others, I'd leave them out. As for pixel_extents, would it not make more sense to expose a function to convert a rectangle from pango units to device units ?
Ok, good. Any ide how it should work? The current versions look suspicious to me (bug 329547). Should they round? floor? what?
Answering myself. I think the most important property that such a function should hold is that touching non-overlaping rectangles should still touch and not overlap after rounded using such a functions. That means the same function should be applied to the start and end coordinates (and not start and length coordinates!) As for the function itself, looking around gdkpango and pango itself, some use PANGO_PIXELS that rounds, others use / PANGO_SCALE which floors. Lets go for rounding. I'll cook a patch for Pango.
The layoutline version should not correct for rotated bounding box. Also, you cannot do tmp_matrix = PANGO_MATRIX_INIT; but must do... static const PangoMatrix unit = PANGO_MATRIX_INIT; tmp_matrix = unit;
This just got alot more serious. It looks like cairo is now enforcing a limit of 2^15 That limits gnumeric to about 2500 rows which is a significant regression. I'll see if this can be ameliorated in foocanvas but you'll need to check TreeView .
It used to be that cairo converted to fixed point *after* applying the "device offset", so it would never handle coordinates outside the X coordinate space. Did that change?
(In reply to comment #18) > It used to be that cairo converted to fixed point *after* applying the > "device offset", so it would never handle coordinates outside the > X coordinate space. Did that change? Ouch. Yes it did, (as part of some work that mozilla did which pushed the device_offset stuff from the gstate layer down into the surface layer of cairo). Compare _cairo_gstate_user_to_backend to see the difference. I'll look into what it would take to bring the necessary parts back up. Sorry about the regression... -Carl
Created attachment 68186 [details] [review] Move device_transform of path to before floating->fixed conversion Here's a suggested patch to cairo to address this problem. Please test this and let me know how it works for you. Thanks, -Carl
cworth #20 : The patch solves the 2^15 limitation for me. Once it goes in we can downgrade this back to normal. Thanks.
(In reply to comment #21) > cworth #20 : The patch solves the 2^15 limitation for me. Once it goes in we > can downgrade this back to normal. Thanks. Thanks for verifying the fix. I just pushed out a fix for this: http://gitweb.cairographics.org/?p=cairo;a=commit;h=bd92eb7f3c58fdcbe05f67b9a879798246c616bc -Carl PS. I'd mark the bug as resolved, but it appears I don't have permission for that.
cworth #22 : It's not resolved yet. We're just back to where we started with a bound of 2^21.
Jody, can you submit a patch based on Morten's feedback in comment 16? I'm going to add a few convenience functions in Pango for rotated bounds, etc, but they will only be in the next stable release, so we can fix this bug by duplicating code in the mean time.
Created attachment 68521 [details] [review] proposed pango api Proposed new api for pango.
This didn't go into pango yet afaics. Is it going in at some point?
Behdad, did you want to fix this ?
Yes, I'm working on this.
Ok, committed the Pango API: 2007-01-04 Behdad Esfahbod <behdad@gnome.org> Part of Bug 332266 – gdk_draw_layout fails for coordinates >= 2^21 * pango/pango-types.h: * pango/pango-matrix.c: New public API: pango_matrix_transform_distance() pango_matrix_transform_point() pango_matrix_transform_rectangle() pango_matrix_transform_pixel_rectangle() * pango/pango-utils.h: * pango/pango-utils.c: New public API: pango_units_from_double() pango_units_to_double() pango_extents_to_pixels() * pango/pango-layout.c (pango_layout_get_pixel_extents), (pango_layout_line_get_pixel_extents): Use pango_extents_to_pixels(). * pango/pangocairo-fcfont.c: (pango_cairo_fc_font_glyph_extents_cache_init), (compute_glyph_extents): Use pango_units_from_double(). * examples/renderdemo.c (do_output): Use pango_matrix_transform_pixel_rectangle(); * pango/pango.def: * docs/pango-sections.txt: * docs/tmpl/glyphs.sgml: Update. So, the get_rotated_layout_bounds() function (in gdkpango and gtklabel) can be replaced with a pango_matrix_transform_rectangle() followed by a pango_extents_to_pixels(). Working on the gtk+ patch. Test case more than welcome.
Created attachment 79423 [details] [review] patch Actually no new Pango API was needed. Transformed PangoLineLayout's should not be shifted, cause the x,y you pass on is the position of the baseline-left of the line, no matter what the rotation is.
Created attachment 79428 [details] [review] Patch to use pango_matrix_transform_rectangle Patch to use the Pango API added above. Doesn't really have anything to do with this bug. Removes some code.
2007-01-05 Behdad Esfahbod <behdad@gnome.org> * gdk/gdkprivate.h: * gdk/gdkpango.c (gdk_draw_layout_line_with_colors), (gdk_draw_layout_with_colors): * gdk/gdkwindow.c (gdk_window_draw_glyphs_transformed): Avoid overflow when converting coordinates to Pango units. (#332266, Jody Goldberg)
attachment 79428 [details] [review] will be followed in bug 340141.