Bug 178232 - Share logic in InlineTextBox to compute selection rect
Summary: Share logic in InlineTextBox to compute selection rect
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Local Build
Hardware: All All
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on: 178611 178671
Blocks: 175784 178613
  Show dependency treegraph
 
Reported: 2017-10-12 14:12 PDT by Daniel Bates
Modified: 2017-10-23 18:39 PDT (History)
6 users (show)

See Also:


Attachments
Patch (21.53 KB, patch)
2017-10-12 14:38 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (21.57 KB, patch)
2017-10-12 14:40 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (21.53 KB, patch)
2017-10-12 15:09 PDT, Daniel Bates
zalan: review+
Details | Formatted Diff | Diff
Work-in-progress (18.44 KB, patch)
2017-10-23 11:08 PDT, Daniel Bates
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-elcapitan (1.09 MB, application/zip)
2017-10-23 12:09 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews116 for mac-elcapitan (2.06 MB, application/zip)
2017-10-23 12:14 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (1.24 MB, application/zip)
2017-10-23 18:39 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2017-10-12 14:12:10 PDT
Currently each paint routine in InlineTextBox computes the selection rect it will paint using the same algorithm or an algorithm that varies slightly. The slight variations represent simplifications of the algorithm that are specific to what is being painted. For example, InlineTextBox::paintDocumentMarker() takes advantage of the fact that the marker decoration is an underline such that it does not concern itself with overlapping backgrounds as InlineTextBox::paintSelection() needs to do. Therefore, InlineTextBox::paintDocumentMarker() can assume the height of the marker is the height of selected text in the box ignoring overlap (call InlineTextBox::selectionHeight()). In contrast, InlineTextBox::paintSelection() must compute the height taking into account the height of any selected text in the last line of the proceeding block (using RootInlineBox::selectionTopAdjustedForPrecedingBlock()) so as make the illusion that the background for all of the selected text on the page was painted in one continuous paint operation as opposed to being painted piecewise. We should share more code.
Comment 1 Radar WebKit Bug Importer 2017-10-12 14:12:27 PDT
<rdar://problem/34963452>
Comment 2 Daniel Bates 2017-10-12 14:38:18 PDT
Created attachment 323564 [details]
Patch
Comment 3 Daniel Bates 2017-10-12 14:40:34 PDT
Created attachment 323566 [details]
Patch
Comment 4 Daniel Bates 2017-10-12 15:09:30 PDT
Created attachment 323575 [details]
Patch
Comment 5 Daniel Bates 2017-10-12 15:52:58 PDT
Comment on attachment 323575 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=323575&action=review

> Source/WebCore/rendering/InlineTextBox.cpp:677
> +    context.fillRect(snapRectToDevicePixelsWithWritingDirection(selectionRect, renderer().document().deviceScaleFactor(), direction() == LTR), c);

Will substitute isLeftToRightDirection() for "direction() == LTR" before landing.

> Source/WebCore/rendering/InlineTextBox.cpp:699
> +    context.fillRect(snapRectToDevicePixelsWithWritingDirection(selectionRect, renderer().document().deviceScaleFactor(), direction() == LTR), color);

Ditto.
Comment 6 zalan 2017-10-19 11:08:36 PDT
Comment on attachment 323575 [details]
Patch

This patch could use a bit of 'auto'.
Comment 7 Daniel Bates 2017-10-19 11:55:35 PDT
Committed r223699: <https://trac.webkit.org/changeset/223699>
Comment 8 Daniel Bates 2017-10-23 10:28:23 PDT
Reverted r223699 for reason:

Caused regressions with right-to-left text selection and painting of markers in flipped writing mode and in overlapping lines. Will investigate offline.

Committed r223840: <https://trac.webkit.org/changeset/223840>
Comment 9 Daniel Bates 2017-10-23 11:08:08 PDT
Created attachment 324568 [details]
Work-in-progress

Incorporates the source code change from the patch for bug #178611, attachment #324477 [details].
Comment 10 Build Bot 2017-10-23 12:09:08 PDT
Comment on attachment 324568 [details]
Work-in-progress

Attachment 324568 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4960534

New failing tests:
platform/mac/fast/dom/character-index-for-point.html
fast/repaint/selection-ruby-rl.html
editing/spelling/spelling-markers-in-overlapping-lines.html
editing/spelling/spelling-markers-in-overlapping-lines-large-font.html
fast/dom/Range/getClientRects.html
Comment 11 Build Bot 2017-10-23 12:09:10 PDT
Created attachment 324574 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 12 Build Bot 2017-10-23 12:14:15 PDT
Comment on attachment 324568 [details]
Work-in-progress

Attachment 324568 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4960488

New failing tests:
platform/mac/fast/dom/character-index-for-point.html
fast/repaint/selection-ruby-rl.html
editing/spelling/spelling-markers-in-overlapping-lines.html
fast/dom/Range/getClientRects.html
editing/spelling/spelling-markers-in-overlapping-lines-large-font.html
Comment 13 Build Bot 2017-10-23 12:14:19 PDT
Created attachment 324575 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 14 Build Bot 2017-10-23 18:39:56 PDT
Comment on attachment 324568 [details]
Work-in-progress

Attachment 324568 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4964592

New failing tests:
fast/repaint/selection-ruby-rl.html
fast/dom/Range/getClientRects.html
Comment 15 Build Bot 2017-10-23 18:39:57 PDT
Created attachment 324625 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6