Summary: | [cairo] Entering text into forms on github.com creates a trapezoid artifact | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> | ||||||||||||||
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | aperez, bugs-noreply, cedric.bellegarde, cgarcia, don.olmstead, Hironori.Fujii, mcatanzaro, svillar, webkit-bug-importer, yoon | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | PC | ||||||||||||||||
OS: | Linux | ||||||||||||||||
URL: | http://github.com | ||||||||||||||||
See Also: |
https://bugzilla.gnome.org/show_bug.cgi?id=759376 https://bugs.freedesktop.org/show_bug.cgi?id=100722 https://bugs.webkit.org/show_bug.cgi?id=170988 |
||||||||||||||||
Attachments: |
|
Description
Michael Catanzaro
2013-12-21 13:24:06 PST
Created attachment 219855 [details]
screenshot of artifact
Just kidding: the blue trapezoid appeared to disappear immediately before the screenshots were taken, but it actually does appear in the screenshots. Here's one.
Can also be gray (GNOME #759376). Created attachment 290312 [details]
another artifact with a triangular shape
Can confirm this issue but I think composent is wrong as it doesn't happen on Safari. Should be moved to GTK. Created attachment 306662 [details]
If you see the red when you input the space, it faiils.
This bug happens when we are drawing the border of the inline box which has a
complex border (such as dashed, dotted..) with a border radius property.
When we made a change inside of text box, for example, typing space, we are
going to invalidate the area of the inline box.
While rendering decorations of the render box,
RenderBoxModelObject::paintBorder defines a clip out region to prepare the inner
border to avoid the background bleeding out behind the border.
We are facing a problem from now on.
The RenderBoxModelObject is going to make a clip to render each side of the
border area. But when we apply this clipping to the cairo context, it seems
cairo context looses the previous clipped out area, which was defined to avoid
bleeding.
It is okay when we draw dashed border side since it is a stroke. But when we
draw a solid border side, it uses fill operation which would fill the whole
quad of the border side with a border color.
I tried to make simple cairo problem to reproduce the problem but it is not so
successful so far.
It is a bug of Cairo while handling clipping regions. I made unit test cases for Cairo and made a bug: https://bugs.freedesktop.org/show_bug.cgi?id=100722 I also made a small optimization patch which avoids the this condition, https://bugs.webkit.org/show_bug.cgi?id=170988 But it just removes this problem when there is no scrolling area. OK, thanks Yoon! It looks like there are no further changes to be made in WebKit, so closing this now. Why not keeping this open till the cairo bug is not fixed? I find it interesting for tracking purposes. We could use this very same bug later when updating the dependencies with the cairo version. Please no, only keep WebKit bugs open if a source code change to WebKit is required. The GitHub issue is fixed anyway. (In reply to Michael Catanzaro from comment #9) > Please no, only keep WebKit bugs open if a source code change to WebKit is > required. The GitHub issue is fixed anyway. That might be you, that is not a general policy and different people do different things. Secondly, I don't know what you mean with fixed, but the Cairo patch is still waiting for review so still far from landing and being added to a release we could depend on. > Secondly, I don't know what you mean with fixed, but the Cairo patch is
> still waiting for review so still far from landing and being added to a
> release we could depend on.
Yoon committed a patch to work around the problem last week.
Just upgraded to webkit2gtk 2.16.2 and issue is always present. Happen less I think but happen everytime I post a comment on github. (In reply to Cédric Bellegarde from comment #12) > Just upgraded to webkit2gtk 2.16.2 and issue is always present. > > Happen less I think but happen everytime I post a comment on github. The shortlist of patches merged into the 2.16.2 release is here: https://trac.webkit.org/wiki/WebKitGTK/2.16.x It does not seem to include the fix for this bug. (CC'ing Carlos García, who usually makes our releases.) Carlos, do you think it would be feasible to include this fix in an upcoming 2.16.x release? (In reply to Adrian Perez from comment #14) > (CC'ing Carlos García, who usually makes our releases.) > > Carlos, do you think it would be feasible to include this fix in > an upcoming 2.16.x release? (In reply to Cédric Bellegarde from comment #12) > Just upgraded to webkit2gtk 2.16.2 and issue is always present. > > Happen less I think but happen everytime I post a comment on github. I patched to skip border paintings in certain conditions and that patch reduces artifacts a bit. But to fix this problem, we need to fix a clipping bug in Cairo or we need to change a way to render box decorations, which is quite different scope of work. This bug does not include a "fix" for this bug. (In reply to Adrian Perez from comment #13) > (In reply to Cédric Bellegarde from comment #12) > > Just upgraded to webkit2gtk 2.16.2 and issue is always present. > > > > Happen less I think but happen everytime I post a comment on github. > > The shortlist of patches merged into the 2.16.2 release is here: > > https://trac.webkit.org/wiki/WebKitGTK/2.16.x No no no. This is not the list of patches merges, this is the list of patches people request me to merge. You should add commits there once they land in trunk and you think they should be backported to stable branch. The list of commits actually merged is this: http://trac.webkit.org/log/webkit/releases/WebKitGTK/webkit-2.16 > It does not seem to include the fix for this bug. And it does include the workaround for this: http://trac.webkit.org/changeset/216373/webkit/releases/WebKitGTK/webkit-2.16 (In reply to Michael Catanzaro from comment #11) > Yoon committed a patch to work around the problem last week. Anyway it clearly doesn't work; the trapezoid appears just like it did five years ago. The bug has migrated: https://gitlab.freedesktop.org/cairo/cairo/issues/297 I wonder if this affects WinCairo? Created attachment 356938 [details] Screenshot of WinCairo WebKitLegacy (In reply to Michael Catanzaro from comment #17) > I wonder if this affects WinCairo? It's relatively easy to reproduce within WinCairo WK1 window with Comment 5 test case and GitHub textarea. It's much difficult to reproduce within WinCairo WK2 window, but I'm able to reproduce it with GitHub textarea. I'm not so sure this is a cairo bug, even though it could be fixed in cairo. I think it's actually a limitation, since the case is handled in the code. See: https://gitlab.freedesktop.org/cairo/cairo/blob/master/src/cairo-clip-polygon.c#L49 _cairo_clip_get_polygon() calls can_convert_to_polygon() and returns early with CAIRO_INT_STATUS_UNSUPPORTED if it fails. can_convert_to_polygon() simply checks that all paths in the chain have the same antialiasing. So, mixing antialiasing modes in the same clip is not actually supported by cairo. In the case of rectangle clips we are already ignoring the current antialiasing to not do any antialiasing. We could do the opposite for clips receiving a path, we want to enforce antialiasing in that case since the path might contain curves. Doing that we ensure all calls to clip with a path use the same antialiasing, which is the case of the github bug. Created attachment 372047 [details]
Patch
Comment on attachment 372047 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372047&action=review Thanks so much for fixing this! You forgot the ChangeLog entry. > Source/WebCore/platform/graphics/cairo/CairoOperations.cpp:1276 > - cairo_antialias_t savedAntialiasRule = cairo_get_antialias(cr); > - cairo_set_antialias(cr, CAIRO_ANTIALIAS_NONE); > - cairo_clip(cr); > +// cairo_antialias_t savedAntialiasRule = cairo_get_antialias(cr); > +// cairo_set_antialias(cr, CAIRO_ANTIALIAS_NONE); > +// cairo_clip(cr); > + doClipWithAntialias(cr, CAIRO_ANTIALIAS_NONE); You also forgot to remove the original code. Try again. :) (In reply to Michael Catanzaro from comment #21) > Comment on attachment 372047 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=372047&action=review > > Thanks so much for fixing this! > > You forgot the ChangeLog entry. > > > Source/WebCore/platform/graphics/cairo/CairoOperations.cpp:1276 > > - cairo_antialias_t savedAntialiasRule = cairo_get_antialias(cr); > > - cairo_set_antialias(cr, CAIRO_ANTIALIAS_NONE); > > - cairo_clip(cr); > > +// cairo_antialias_t savedAntialiasRule = cairo_get_antialias(cr); > > +// cairo_set_antialias(cr, CAIRO_ANTIALIAS_NONE); > > +// cairo_clip(cr); > > + doClipWithAntialias(cr, CAIRO_ANTIALIAS_NONE); > > You also forgot to remove the original code. Try again. :) Oops, I actually submitted the wrong diff, let me upload the right one. Created attachment 372057 [details]
Patch
Committed r246431: <https://trac.webkit.org/changeset/246431> |