Closed Bug 374980 (compositor) Opened 15 years ago Closed 1 year ago

Implement Compositor

Categories

(Core :: Layout, task)

task
Not set
normal

Tracking

()

RESOLVED INACTIVE

People

(Reporter: roc, Unassigned)

References

(Depends on 1 open bug, Blocks 4 open bugs)

Details

Attachments

(2 files)

Attached patch WIPSplinter Review
This is very incomplete and does not build. I just want it backed up somewhere.
Attachment #259386 - Attachment is patch: true
Attachment #259386 - Attachment mime type: application/octet-stream → text/plain
What this patch has so far:
-- view hierarchies are unified
-- the prescontext for the root of the Gecko window tree is an nsRootPresContext
-- nsRootPresContext implements the compositor functionality
-- nsRootPresContext manages one or more nsWindowControllers --- usually just one but additional ones for popups
-- nsWindowControllers manage popups
-- move scrolling implementation to layout
-- nsScrollPortView just forwards its API calls to the scrollframe
-- scrolling is handled by a RequestCopyArea method on frames, which works a lot like Invalidate
-- scroll requests are queued along with invalidates in the nsRootPresContext
-- the compositor repaints according to the frame rate unless RequestUrgentUpdate is called; all painting is asynchronous
-- before processing paints and scrolls, we run queued "pre-update" events
-- smooth scrolling reimplemented as an animation source; we update the scrollbar positions when (and only when) we're about to paint
-- scrolling's "onscroll" and scrollport events run as pre-update events so script can modify the DOM before painting
-- defined a new nsIWidget::UpdateWindow API that simultaneously repositions plugins, applies scroll operations, and repaints invalid window areas
-- some gfx utility methods
-- removed existing paint suppression/batching code in various places. It's mostly no longer needed. In some places we will need to put something else back.
Hmm, it'd probably be a good idea to split out as many of the changes as possible.  I think the easy bits to split out are the parts involving changing the nsIScrollPositionListener API, changing nsIScrollableViewProvider to nsIScrollableFrameProvider, adding the new APIs to nsIScrollableFrame, and changing all the users outside of the scroll-frame code of nsIScrollableView to nsIScrollableFrame.

The WIP doesn't seem to respect the current behavior of nsIScrollableViewProvider; for some frames, it provides a view associated with a child scrollframe.  That's why it should probably be changed to nsIScrollableFrameProvider rather than just being zapped (or we could add a virual method on nsIFrame...).

I also might add that the attached version has two copies of nsGfxScrollFrame.cpp, and seems to be missing nsObjectFrame.cpp; I'm assuming that's because the patch isn't finished yet.
(In reply to comment #4)
> Hmm, it'd probably be a good idea to split out as many of the changes as
> possible.

I know. I spent an awfully long time trying to figure out how to break all this work down into separate pieces but ultimately that seemed so hard, thanks to cyclic dependencies, it'd be easier to do most of it in one go.

> I think the easy bits to split out are the parts involving changing
> the nsIScrollPositionListener API, changing nsIScrollableViewProvider to
> nsIScrollableFrameProvider, adding the new APIs to nsIScrollableFrame,

Moving nsIScrollPositionListener from views to layout may be possible as a pre-patch, yeah. I should probably try to do that. But the rest would be tricky without moving scrolling from views to layout, and that's tied intimately into the rest of the patch.

> and
> changing all the users outside of the scroll-frame code of nsIScrollableView to
> nsIScrollableFrame.

I'm actually not doing that, I've left nsIScrollableView around forwarding everything to the frame, so that we have to change less code. Of course ultimately we do want to change all the users to use the frame.

> The WIP doesn't seem to respect the current behavior of
> nsIScrollableViewProvider; for some frames, it provides a view associated with
> a child scrollframe.  That's why it should probably be changed to
> nsIScrollableFrameProvider rather than just being zapped (or we could add a
> virual method on nsIFrame...).

I'll look into that.

> I also might add that the attached version has two copies of
> nsGfxScrollFrame.cpp, and seems to be missing nsObjectFrame.cpp; I'm assuming
> that's because the patch isn't finished yet.

It's actually because I goofed up some incremental updates to the patch as I was reviewing it. Nothing's lost, I have it in a dedicated tree.
Blocks: 372039
Note that this is not going to make Gecko 1.9.
Does that mean that Gecko 1.9 will come out with bug 324819 unfixed? That would be a terrible regression from 1.8!
(In reply to comment #7)
> Does that mean that Gecko 1.9 will come out with bug 324819 unfixed? That would
> be a terrible regression from 1.8!

No, because that bug has the blocking flag 1.9+ set.
But will it be possible without this one? The dependence chain of bug 324819 leads up to this one.
Yes, it will be possible.
Bug 130078 depends on this one. Could it be done in 1.9? Or when?
No way for 1.9. Top priority after 1.9.
Blocks: 400925
Blocks: 420608
Blocks: 418351
Blocks: 433646
This is a major issue in 1.9!!! There are a large number of canned applications which are broken because of Bug 130078. These include, but are not limited to, XWiki and OpenNMS. These applications are critical to our company and to many others. Additionally, there are countless other sites which use the "overflow: auto" property and will be broken on the initial release of Firefox 3.0. This will be VERY bad for end users.
Flags: blocking1.9?
Deven, please file new bugs for whatever problems you see in Firefox 3 with XWiki and OpenNMS. Also try to mention some of the other sites that also have problems in that bug.
Deven if we can get specific bugs for any incompatibility issues we will look at those for FF3 - but this bug is a feature request for a major re-work of parts of our graphics pipeline which cannot be done in FF3.
Flags: blocking1.9? → blocking1.9-
This bug is blocking bug #215055 and that has the specific errors and test cases attached.
Flags: blocking1.9- → blocking1.9?
Read comment #12. This is NEVER going to happen for 1.9 so there is no point requesting it as a blocker.
Flags: blocking1.9?
That is all well and good, as I understand that this is a difficult recode. So, there needs to be a fix that does not require the compositor. That fix needs to be a stopper for 1.9. If the compositor cannot be complete for 1.9, then what can we do to fix the large block elements problem?
Deven, bug 215055 is not a regression - it's there since the beginnings of Mozilla.
Blocks: 452048
Alias: compositor
Blocks: 457862
Could we get a status update on this fix?  Is it at all likely to make it into 3.2 or are we waiting for some much later release?  I understanding reworking the whole page composition process is a fairly large change and will take a while to do.
Blocks: 481128
Lot of performance issues associated with fixed background. It's because in such cases browser need to completely redraw all page for each scrolling event.

What if compositor will draw such pages in two window mode (in modern OSes). First window will draw fixed background and never will be scrolled. And second window above will draw page content with translucent background. All composition will be made by OS using hardware resources and - it will be very fast on Vista, Mac OS X and Linux with Composite WMs
> First window will draw fixed background and never will be scrolled.
Instead "never" it can be scrolled only when scrolling is finished (Button Release event)
Any updates on this? Perhaps a newer WIP since the 2007 one? Is this now targeted @ 1.9.2/1.9.3, or is this a 2.0 goal?
A big chunk landed on trunk this week in bug 339548 and bug 352093. I'm working on the rest.
Depends on: 465216
Depends on: 510008
Wow, this is a massive amount work Robert, how much more do you expect to have to do?
No longer blocks: 130078
Depends on: 130078
Assignee: roc → nobody
Blocks: 318474
Blocks: 607346
Blocks: 614562
No longer blocks: 614562
No longer blocks: 420608

Mass-removing myself from cc; search for 12b9dfe4-ece3-40dc-8d23-60e179f64ac1 or any reasonable part thereof, to mass-delete these notifications (and sorry!)

No activity for a long time, closing

Status: NEW → RESOLVED
Type: defect → task
Closed: 1 year ago
Resolution: --- → INACTIVE
You need to log in before you can comment on or make changes to this bug.