WebKit Bugzilla
You need to log in before you can comment on or make changes to this bug.
Created an attachment (id=46170) [details] Test case This bug was discovered while investigating https://bugs.webkit.org/show_bug.cgi?id=32757 Open the test case and try to drag the third star. Note that small grey dots are left behind as the star is dragged. I suspect that there may be a problem where strokeBoundingBox() is not quite big enough in this odd case where the right-most point of the shape is a blank spot in the dash stroke. Of course, I could be wrong too :-) Also, please note that the test case of course is manual, but if moved into the fast/repaint directory, it will be an automated test too.
(In reply to comment #0) > Created an attachment (id=46170) [details] [details] > Test case > > This bug was discovered while investigating > https://bugs.webkit.org/show_bug.cgi?id=32757 > > Open the test case and try to drag the third star. Note that small grey dots > are left behind as the star is dragged. I suspect that there may be a problem > where strokeBoundingBox() is not quite big enough in this odd case where the > right-most point of the shape is a blank spot in the dash stroke. Of course, I > could be wrong too :-) > > Also, please note that the test case of course is manual, but if moved into the > fast/repaint directory, it will be an automated test too. Yes, the problem was in strokeBoundingBox. The strokeRect must not contain the boundingBox of the path. We need the union of both.
Created an attachment (id=46204) [details] fix of strokeBoundingBox in RenderPath This is the fix for the problem. I'm not sure how to make a regression test for this bug. I think it just needs a copy of Beth's shadow repaint test without shadows :-D
Dirk, the patch looks great! The test case that I attached to this bug is actually already layout test, but only it you add it to the fast/repaint directory since it relies on some javascript that is stored in fast/repaint/resources. If you add this test case there and attach a new patch, I will r+. Also, the automated version of the test case that I attached is pretty subtle; it just leaves behind two pixels of grey that shouldn't be there. I don't think there is a way to make it more obvious with our current repaint test mechanism, but if this is something that concerns you, you could change the color of the stars to something that will be more obvious, like black instead of grey, and you could always add some text to the test about how it works. None of this is really necessary though.
(In reply to comment #3) >The test case that I attached to this bug is > actually already layout test, but only it you add it to the fast/repaint This should say, "The test case that I attached to this bug is actually already a layout test, but only IF you add it to the fast/repaint…"
> it just leaves behind two pixels of grey that shouldn't be there Does that mean that the test is ineffective due to to run-webkit-tests tolerating small differences by default?
(In reply to comment #5) > > it just leaves behind two pixels of grey that shouldn't be there > > Does that mean that the test is ineffective due to to run-webkit-tests > tolerating small differences by default? Good question. I haven't actually run it through DumpRenderTree, so I am not sure.
Created an attachment (id=46434) [details] fix of strokeBoundingBox in RenderPath Same patch with simple test of the repaint calculation. I'm not sure if other tests are involved by this patch. My Mac Os system is currently broken. Could some one run webkit-tests with this patch please?
Created an attachment (id=46475) [details] fix of strokeBoundingBox in RenderPath Got my system running again. This is the same patch as above. I was surprised how many tests are affected by this changes. I checked the results and the repaint rect got always bigger, and are correct according to the tests output.
(From update of attachment 46475 [details]) r=me
(From update of attachment 46475 [details]) Thanks for the quick review :-)
(From update of attachment 46475 [details]) Rejecting patch 46475 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1 Running build-dumprendertree Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 11919 test cases. fast/repaint/moving-shadow-on-container.html -> failed Exiting early after 1 failures. 7967 tests run. 129.29s total testing time 7966 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 3 test cases (<1%) had stderr output Full output: http://webkit-commit-queue.appspot.com/results/184578
Updating the failing tests. Will upload a second patch that fixes the missing result updates.
Created an attachment (id=46510) [details] forgot two results in previous patch forgot two results in previous patch.
landed in r53207. Closing bug.