Bug 66254 - [CSSRegions] Make regions repaint properly.
Summary: [CSSRegions] Make regions repaint properly.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords:
Depends on:
Blocks: 57312
  Show dependency treegraph
 
Reported: 2011-08-15 14:40 PDT by Dave Hyatt
Modified: 2011-09-23 13:25 PDT (History)
4 users (show)

See Also:


Attachments
Work in progress (just need tests) (7.13 KB, patch)
2011-08-15 14:40 PDT, Dave Hyatt
no flags Details | Formatted Diff | Diff
Patch (322.42 KB, patch)
2011-08-16 11:39 PDT, Dave Hyatt
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Hyatt 2011-08-15 14:40:11 PDT
Make sure regions do proper repainting when their content changes.
Comment 1 Dave Hyatt 2011-08-15 14:40:58 PDT
Created attachment 103957 [details]
Work in progress (just need tests)
Comment 2 Simon Fraser (smfr) 2011-08-15 14:44:53 PDT
Comment on attachment 103957 [details]
Work in progress (just need tests)

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

> Source/WebCore/rendering/RenderView.h:251
>      bool m_pageLogicalHeightChanged;
>      LayoutState* m_layoutState;
>      unsigned m_layoutStateDisableCount;
> +    bool m_hasRenderFlowThreads;

Put this next to the other bools to reduce padding.
Comment 3 Mihnea Ovidenie 2011-08-16 10:52:53 PDT
This will likely also fix https://bugs.webkit.org/show_bug.cgi?id=66199.
Comment 4 Dave Hyatt 2011-08-16 11:39:25 PDT
Created attachment 104073 [details]
Patch
Comment 5 Simon Fraser (smfr) 2011-08-16 11:45:42 PDT
Comment on attachment 104073 [details]
Patch

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

> Source/WebCore/rendering/RenderFlowThread.cpp:305
> +        // We only have to issue a repaint in this region if the region rect intersects the repaint rect.
> +        LayoutRect flippedRegionRect(region->regionRect());
> +        flipForWritingMode(flippedRegionRect); // Put the region rect into physical coordinates.

Can we assume that the rect will only intersect some contiguous set of regions? That might avoid having to iterate through all regions for each repaint, which is not terribly efficient.

> LayoutTests/platform/mac/fast/repaint/line-flow-with-floats-in-regions-expected.txt:1
> +layer at (0,0) size 800x600

Can the tests avoid the use of so much text, to reduce spurious pixel diffs?
Comment 6 Dave Hyatt 2011-09-23 13:25:01 PDT
Fixed ages ago. Just forgot to close.