GNOME Bugzilla – Bug 728636
textoverlay: cuts off right edge italicised text
Last modified: 2015-08-16 13:36:33 UTC
Created attachment 274794 [details] Pitivi screenshot illustrating the cutting of the text See the attached screenshot, the outline and the shadow on the right side of the word are cut at least in some cases (big bold italic text). Might be a gst-editing-services/ges/ges-title-source.c problem or a textoverlay problem.
Any steps to reproduce this issue? Happens every time in pitivi?
Created attachment 276529 [details] Left side can also happen to be cut Steps to reproduce (in Pitivi): - Add a title with "URW Gothic L Book Oblique" (or "Nimbus Sans L Bold Italic") size 72, text "test", notice the right side is cut. OR - Add a title with "Nimbus Roman No9 L Bold Italic" size 72, text "test", notice the left side is cut.
Can be reproduced with: gst-launch-1.0 videotestsrc ! video/x-raw,width=1280,height=720 ! textoverlay font-desc='Nimbus Sans L, 72' text='<b><i>test</i></b>' ! ximagesink
It doesn't seem to happen with all fonts of similar size and itacility. This looks fine: gst-launch-1.0 videotestsrc num-buffers=100 pattern=3 ! video/x-raw,width=1280,height=720 ! textoverlay text='<b><i>test</i></b>' font-desc='Sans, 72' ! ximagesink So maybe a bug in the font, not describing its bounding rectangle correctly. I'm not sure how to check that.
Created attachment 299877 [details] "test" showing the shadow of the second t cut How exactly is the width of the text calculated? Either the logic is broken or the input data. We should start looking at the code? BTW, even with Sans 72 the shadow is cut, see the attached screenshot produced with the command you mentioned. The shadow of the first t is larger than the shadow of the last t.
Oh, indeed, a small amount is cut off. That does indeed point to the code again. I'll see if I can ferret it out.
Created attachment 299943 [details] [review] fix bounding box
Comment on attachment 299943 [details] [review] fix bounding box >The maximum inked extent is >the width of the inked extent plus the offset it starts at. > >It's possible that negative offsets would require further adjustment >to the calculation, if a test case is found. > >- width = (logical_rect.width + overlay->shadow_offset) * scalef; >+ width = >+ (int) ceil ((ink_rect.width + ink_rect.x + >+ overlay->shadow_offset) * scalef); I think you're onto something here. I agree that we should be looking at the ink rectangle not the logical rectangle to calculate the render surface width/height in pixels. However, I'm not sure about the + ink_rect.x (which is what you hinted at in your commit message too I believe?) Take this example here for instance: https://mail.gnome.org/archives/gtk-devel-list/2001-August/msg00325.html where x is negative. I wonder if we should just take ink_rect.width and ignore ink_rect.x ? I think in case of right-to-left text x may also be larger than the rectangle width or just below the width, in which case one may want to handle it differently again to get the actual dimensions (if one was to take it into account), but you'd think the point of 'width' would be to provide the full width of the ink rectangle.
In the case of 0 offset, the code is trivially correct. In the case of a positive offset, the ink spans "ink width", and starts at an offset of "ink offset" from the textoverlay origin, so offset needs adding for the "textoverlay width" to encompass all the ink (thre will be blank space on the left of the ink, which gets counted in that "textoverlay width"). In the case of a negative offset, I'm not sure where the textoverlay origin is. The code as it is would give us a clipped ink, so is still wrong, but I'd need a test case to work out in what direction is needs extending to encompass the extent of the ink. If the X offset from which textoverlay calculates its own rectangle is shifted to the left in that case, then the "ink offset" should not be added. But textoverlay does not seem to shift its own origin, otherwise we would not have had that bug in the first place.
Ok, I guess it's just more complicated then that. Just added some debug log to print the extents. gst-launch-1.0 videotestsrc ! video/x-raw,width=1280,height=720 ! textoverlay font-desc='Bitstream Charter, 72' text='<b><i>g</i></b>' ! ximagesink Gives me, Extents of "<b><i>g</i></b>" is ink (-3, 47) 55x68 and logical (0, 0) 50x117 Regardless how big you make the surface, the left part of the g will always be clipped unless you also translate the matrix the opposite way. Just doing that miss-align the text, so we have to compensate when doing the placement. ink_rect.width though always seems sufficient to fit the text.
Created attachment 308026 [details] [review] basetextoverlay: Fix clipping issues This patch uses the ink rectangle in order to compute the size of the surface require to render. It also correctly compute the transformation matrix as the ink_rect position might not be at 0, 0. Additionally, shadow_offset and outline_offset (which is in fact the diameter of a dot, not a really an offset) is now taken into account. Redundant matrix operation has been removed for the vertical rendering. Take note that the matrix operation in cairo are excuted in reverse order.
Created attachment 308027 [details] [review] basetextoverlay: Don't prevent placing vertical text We could not control the alignment at all when we had vertical text. This limitation was introduced when Composition meta was added. This limitation is arbitrary and should be removed. The vertical render was also not correct, with text going outside it's bounding box.
The second patch is arguably an ABI break, it change the default placement (to the default alignment) when setting vertical-render=1. As Tim find out, it has been like that since the property has been added (in a patch called "pango: Use pango-cairo instead of pango-ft2", :-S). Feel free to reject, it's not really related to this bug. Note that the placement after this patch is not typographically correct (not that it really was before). But the render will always be complete at least, and we will stop over allocating when you have a series of small caps letters.
Comment on attachment 308027 [details] [review] basetextoverlay: Don't prevent placing vertical text I think in case of vertical render we should still default to top-right, it's just what makes the most sense, so perhaps we can find a middle way where we default to that when vertical render is enabled and then people can still override it via setting properties? (Don't think we need to worry too much about restoring old defaults when it's disabled etc.) Also, this was not introduced when composition meta was added but has been there since the very beginning as far as I can tell, see: http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/ext/pango/gsttextoverlay.c?id=7608c31516464783fe4b2194775960cfe2bc2cd0 http://cgit.freedesktop.org/gstreamer/gst-plugins-base/tree/ext/pango/gsttextoverlay.c?id=7608c31516464783fe4b2194775960cfe2bc2cd0#n1478
Noted, will rewrite the commit message. I'll also ensure the default changes if vertical-render is set. I found another bug in the vertical. It does not correctly deal with the auto wrapping, as it will clip to the width rather then height. About the non-vertical part, you asked on IRC what the placement look likes. There of course a small shift, but I think it's acceptable. Here's an example. https://people.collabora.com/~nicolas/TextBefore.png https://people.collabora.com/~nicolas/TextAfter.png The shift down/right matches the amount of the font that was clipped before. So I think this is correct. Note that TextAfter denote a small clipping. I'll fix tomorrow. It's the outline_offset, in with and height, both side of the outline need to be counted, while in translation, only 1 side count.
Created attachment 308105 [details] [review] basetextoverlay: Fix clipping issues This patch uses the ink rectangle in order to compute the size of the surface require to render. It also correctly compute the transformation matrix as the ink_rect position might not be at 0, 0. Additionally, shadow_offset and outline_offset (which is in fact the diameter of a dot, not a really an offset) is now taken into account. Redundant matrix operation has been removed for the vertical rendering. Take note that the matrix operation in cairo are excuted in reverse order.
Created attachment 308106 [details] [review] basetextoverlay: Use the extents rectangle for positioning the extents rectangle is what you need to know to properly position a buffer that has been rendered in a surface of the ink rectangle size. This patch make the placement on par with the placement we had before without having to over allocate. This patch also enable placement for vertical rendering. Note that the halginement, valighment and line-alignment default are set to the previous default when this property is set. This is for backward compatibility, you can change the value after setting vertical render.
Attachment 308105 [details] pushed as 7569a2e - basetextoverlay: Fix clipping issues Attachment 308106 [details] pushed as d4759f0 - basetextoverlay: Use the extents rectangle for positioning
Enjoy ! * no more over allocation * no more clipping * Better vertical render support