[FIX]Users of EndUpdateViewBatch(NS_VMREFRESH_DEFERRED) probably don't need it

RESOLVED FIXED in mozilla1.8alpha5

Status

()

P3
normal
RESOLVED FIXED
14 years ago
2 months ago

People

(Reporter: roc, Assigned: bzbarsky)

Tracking

Trunk
mozilla1.8alpha5
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Followon from bug 262760. A number of callers to
EndUpdateViewBatch(NS_VMREFRESH_DEFERRED) probably don't really need that
semantics (which is to delay all painting until an Invalidate PLEvent is
processed). Mostly they probably just want NS_VMREFRESH_NO_SYNC which will paint
whenever the platform toolkit decides to paint.
(Assignee)

Comment 2

14 years ago
Comment on attachment 161620 [details] [diff] [review]
Fix

This fixes the callers that are really just trying to prevent reflow-triggered
or hand-triggered invalidates from being applied one at a time to use NO_SYNC. 
I added a comment to the nsCSSFrameConstructor code and left nsDocumentViewer
well enough alone (I can't tell why that code even exists, really, so...)

One thing I wonder about.  Do we need to worry about reflow changing widget
dimensions so the invalidates that got deferred won't invalidate the right
areas?	Or would the widget size change automatically invalidate whatever needs
invalidating?
Attachment #161620 - Flags: superreview?(roc)
Attachment #161620 - Flags: review?(roc)
(Assignee)

Updated

14 years ago
Priority: -- → P3
Summary: Users of EndUpdateViewBatch(NS_VMREFRESH_DEFERRED) probably don't need it → [FIX]Users of EndUpdateViewBatch(NS_VMREFRESH_DEFERRED) probably don't need it
Target Milestone: --- → mozilla1.8alpha5
> Or would the widget size change automatically invalidate whatever needs
> invalidating?

The view size change should repaint the difference in areas.
(Assignee)

Comment 4

14 years ago
> The view size change should repaint the difference in areas.

Yes, but the dirty area we have stored on the view will be relative to the old
widget rect, not the new one.  So when we go to actually invalidate, we will
invalidate the wrong area, no?
I looked into this and I think we're OK. nsViewManager::UpdateWidgetArea
distributes invalidation over the views with widgets which cover a particular
area of the screen. When you move a view we invalidate the union of the old and
new areas via the parent view (which will in turn call UpdateWidgetArea to
distribute the invalidation over other views with widgets). I think this
guarantees that the correct part of the screen will eventually be updated,
although a proof would be tricky and require induction over the depth of the
view in the view tree.

This would be simpler and also more efficient if we accumulated invalid regions
per-view and then did the UpdateWidgetArea logic in ProcessPendingUpdates. Note
that if this is broken then it would already have been visible in the places
where we already used Begin/EndUpdateViewBatch.
Attachment #161620 - Flags: superreview?(roc)
Attachment #161620 - Flags: superreview+
Attachment #161620 - Flags: review?(roc)
Attachment #161620 - Flags: review+
(Assignee)

Comment 6

14 years ago
Fixed.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Component: Layout: View Rendering → Layout: Web Painting
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.