Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor caret painting logic #10049

Conversation

mwyrzykowski
Copy link
Contributor

@mwyrzykowski mwyrzykowski commented Feb 13, 2023

e314f57

Refactor caret painting logic
https://bugs.webkit.org/show_bug.cgi?id=252148
<radar://104975415>

Reviewed by Ryosuke Niwa and Aditya Keerthi.

Continue previous refactorings to the caret painting
logic to allow some additional flexibility.

* Source/WebCore/editing/FrameSelection.cpp:
(WebCore::fillCaretRect):
Pass not just the presentation properties but the entire
animator to allow for additional flexibility.

(WebCore::repaintCaretRectForLocalRect):
(WebCore::repaintCaretForLocalRect):
Allow the animator to influence the repaint rect.

(WebCore::FrameSelection::invalidateCaretRect):
(WebCore::CaretBase::invalidateCaretRect):
(WebCore::FrameSelection::paintCaret):
(WebCore::CaretBase::paintCaret const):
(WebCore::DragCaretController::paintDragCaret const):
* Source/WebCore/editing/FrameSelection.h:
Pass the caret animator instead of just its properties.

* Source/WebCore/rendering/RenderBlock.cpp:
(WebCore::renderCaretInsideContentsClip):
(WebCore::RenderBlock::paint):
(WebCore::RenderBlock::paintObject):
Allow rendering the caret outside the element's clipping rectangle.

Canonical link: https://commits.webkit.org/260775@main

1832b98

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ›  gtk
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk1 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv   πŸ§ͺ mac-wk2
βœ… πŸ›  tv-sim   πŸ§ͺ mac-AS-debug-wk2
  πŸ›  watch βœ… πŸ§ͺ mac-wk2-stress
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch-sim

@mwyrzykowski mwyrzykowski self-assigned this Feb 13, 2023
@mwyrzykowski mwyrzykowski added the Layout and Rendering For bugs with layout and rendering of Web pages. label Feb 13, 2023
@mwyrzykowski mwyrzykowski force-pushed the eng/Refactor-caret-painting-logic branch from 624ed1e to 711cc96 Compare February 13, 2023 20:55
@mwyrzykowski mwyrzykowski force-pushed the eng/Refactor-caret-painting-logic branch from 711cc96 to 56f0933 Compare February 13, 2023 21:29
@mwyrzykowski mwyrzykowski force-pushed the eng/Refactor-caret-painting-logic branch from 56f0933 to 796c63b Compare February 13, 2023 21:56
@mwyrzykowski mwyrzykowski force-pushed the eng/Refactor-caret-painting-logic branch from 796c63b to c2c4eee Compare February 14, 2023 18:37
@mwyrzykowski mwyrzykowski force-pushed the eng/Refactor-caret-painting-logic branch from c2c4eee to 22c59ca Compare February 15, 2023 19:15
@mwyrzykowski mwyrzykowski force-pushed the eng/Refactor-caret-painting-logic branch from 22c59ca to 6d639cf Compare February 16, 2023 04:34
@mwyrzykowski mwyrzykowski force-pushed the eng/Refactor-caret-painting-logic branch from 6d639cf to 4d456e9 Compare February 20, 2023 18:41
@@ -1120,6 +1129,11 @@ void RenderBlock::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
if (pushedClip)
popContentsClip(paintInfo, phase, adjustedPaintOffset);

if (paintInfo.phase == PaintPhase::Foreground && !renderCaretInsideContentsClip()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smfr I want to render the caret outside of the RenderBlock / text field's clipping rectangle. Is there a better way to do this than ignoring the clipping rectangle? Should I paint the caret as part of a different paint phase?

Comment on lines 1133 to 1134
paintCaret(paintInfo, adjustedPaintOffset, CursorCaret);
paintCaret(paintInfo, adjustedPaintOffset, DragCaret);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we factor these two lines into a separate method, since it's duplicated below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I'll make that change

@mwyrzykowski mwyrzykowski force-pushed the eng/Refactor-caret-painting-logic branch from 4d456e9 to 1832b98 Compare February 23, 2023 20:28
@webkit-early-warning-system
Copy link
Collaborator

Starting EWS tests for 1832b98. Live statuses available at the PR page, #10049

@mwyrzykowski mwyrzykowski added the merge-queue Applied to send a pull request to merge-queue label Feb 24, 2023
https://bugs.webkit.org/show_bug.cgi?id=252148
<radar://104975415>

Reviewed by Ryosuke Niwa and Aditya Keerthi.

Continue previous refactorings to the caret painting
logic to allow some additional flexibility.

* Source/WebCore/editing/FrameSelection.cpp:
(WebCore::fillCaretRect):
Pass not just the presentation properties but the entire
animator to allow for additional flexibility.

(WebCore::repaintCaretRectForLocalRect):
(WebCore::repaintCaretForLocalRect):
Allow the animator to influence the repaint rect.

(WebCore::FrameSelection::invalidateCaretRect):
(WebCore::CaretBase::invalidateCaretRect):
(WebCore::FrameSelection::paintCaret):
(WebCore::CaretBase::paintCaret const):
(WebCore::DragCaretController::paintDragCaret const):
* Source/WebCore/editing/FrameSelection.h:
Pass the caret animator instead of just its properties.

* Source/WebCore/rendering/RenderBlock.cpp:
(WebCore::renderCaretInsideContentsClip):
(WebCore::RenderBlock::paint):
(WebCore::RenderBlock::paintObject):
Allow rendering the caret outside the element's clipping rectangle.

Canonical link: https://commits.webkit.org/260775@main
@webkit-commit-queue webkit-commit-queue merged commit e314f57 into WebKit:main Feb 24, 2023
@webkit-commit-queue
Copy link
Collaborator

Committed 260775@main (e314f57): https://commits.webkit.org/260775@main

Reviewed commits have been landed. Closing PR #10049 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Layout and Rendering For bugs with layout and rendering of Web pages.
Projects
None yet
5 participants