Closed
Bug 1025138
Opened 7 years ago
Closed 7 years ago
Integrate DrawTargetTiled into layers.
Categories
(Core :: Graphics: Layers, defect)
Core
Graphics: Layers
Tracking
()
RESOLVED
FIXED
mozilla34
People
(Reporter: nical, Assigned: nical)
References
Details
Attachments
(1 file, 2 obsolete files)
23.09 KB,
patch
|
mattwoodrow
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•7 years ago
|
||
Assignee: nobody → nical.bugzilla
Assignee | ||
Comment 2•7 years ago
|
||
Note that TiledDrawTarget might not perform as well for native theme drawing on Linux because it will fail to borrow a cairo context here: http://dxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxXlibNativeRenderer.cpp?from=gfxXLibNativeRenderer.cpp#151
Attachment #8440026 -
Attachment is obsolete: true
Comment 3•7 years ago
|
||
We could improve the native drawing situation by making the borrow function request an area that it wants to draw to. If that only covers a single tile (something I assume would be fairly common), then we can just borrow the context for that tile.
Assignee | ||
Comment 4•7 years ago
|
||
At the moment this patch seems to have clipping issues. It's not entirely clear to me whether the code in TiledContentClient is not properly tracking valid regions or if the clipping issue is in the DrawTargetTiled itself. When turning paint-flashing on I can see that the whole tile is flashed every time rather than just the paint region, and I think that the paint region in FrameLayerBuilder isn't affect by any region code from TiledContentClient that I could have messed up so i'm inclined to think that something is wrong in the tiled DrawTarget itself. I tested with skia instead of cairo and it has the same problem.
Assignee | ||
Comment 5•7 years ago
|
||
This patch has the clipping fixed that Bas pointed out to me, and postpone unlocking the front and back buffers so that we can avoid Flushing the DrawTarget after copying the front into the back buffer, and avoid creating a new DrawTarget for painting.
Attachment #8440061 -
Attachment is obsolete: true
Comment 6•7 years ago
|
||
Can we get this reviewed and landed soon?
Assignee | ||
Updated•7 years ago
|
Attachment #8440745 -
Flags: review?(matt.woodrow)
Comment 7•7 years ago
|
||
Comment on attachment 8440745 [details] [diff] [review] WIP v3 Review of attachment 8440745 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/TiledLayerBuffer.h @@ +516,5 @@ > > + AsDerived().PostValidate(aPaintRegion); > + for (unsigned int i = 0; i < newRetainedTiles.Length(); ++i) { > + AsDerived().UnlockTile(newRetainedTiles[i]); > + //MOZ_ASSERT(!IsPlaceholder(newRetainedTiles[i]), "invalid tile"); Why is this commented out? ::: gfx/layers/client/TiledContentClient.cpp @@ +660,2 @@ > > + } else if (useSinglePaintBuffer) { if (useSinglePaintBuffer && !gfxPrefs::TiledDrawTargetEnabled()) instead of having the empty branch. @@ +776,5 @@ > + return aTile; > + } > + } > + > + if (gfxPrefs::TiledDrawTargetEnabled()) { This function is enormous now, maybe split this block into a separate function? ::: modules/libpref/src/init/all.js @@ +3767,5 @@ > pref("layers.draw-tile-borders", false); > pref("layers.draw-bigimage-borders", false); > pref("layers.frame-counter", false); > pref("layers.enable-tiles", false); > +pref("layers.tiled-drawtarget.enabled", true); Probably want to remove this for now :)
Attachment #8440745 -
Flags: review?(matt.woodrow) → review+
Comment 8•7 years ago
|
||
Can you land this soon please Nical? I have follow-up changes waiting to land that need this.
Assignee | ||
Comment 9•7 years ago
|
||
adressed some of the comments and landed https://hg.mozilla.org/integration/mozilla-inbound/rev/7aee66d772f4 I looked into making this a bit cleaner but the whole client side stuff needs to be split into clean chuncks so i'll spend some time doing this as a followup (after OMTC sticks at least on windows).
Assignee | ||
Comment 10•7 years ago
|
||
> I looked into making this a bit cleaner but the whole client side stuff
> needs to be split into clean chuncks so i'll spend some time doing this as a
> followup (after OMTC sticks at least on windows).
sorry a bit tired, i meant the whole client side tiling code, so TiledLayerBuffer and TiledContentClient
Comment 11•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7aee66d772f4
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in
before you can comment on or make changes to this bug.
Description
•