Closed Bug 979489 (can-tiles) Opened 6 years ago Closed 6 years ago

Implement SimpleTiledContentClient

Categories

(Core :: Graphics: Layers, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: BenWa, Assigned: vlad)

References

Details

Attachments

(1 file, 4 obsolete files)

SimpleTiledContentClient is a variation on TiledContentClient that doesn't use buffer re-use strategies.
Attached patch WIP (obsolete) — Splinter Review
Blocks: 974152
Android R2 failure:

border-image/border-image-element.html | image comparison (==), max difference: 125, number of differing pixels: 5808

This test compares   { border-image: -moz-element:(#src); } <img id="src" src="diamonds.png">   and { border-image: url("diamonds.png"); }.   The one using -moz-element seems to be using NEAREST when scaled up, and the second seems to be using LINEAR.  There is maybe relevant code:

http://dxr.mozilla.org/mozilla-central/source/gfx/layers/opengl/CompositorOGL.cpp#1117

Or something else that isn't setting LINEAR properly..
No longer blocks: 974152
Attached patch Simple Tiles (obsolete) — Splinter Review
Attachment #8386936 - Attachment is obsolete: true
Attachment #8388649 - Flags: review?(jmuizelaar)
Attached patch Simple tiles v4Splinter Review
Attachment #8388649 - Attachment is obsolete: true
Attachment #8388660 - Flags: review?(jmuizelaar)
Comment on attachment 8388660 [details] [diff] [review]
Simple tiles v4

Review of attachment 8388660 [details] [diff] [review]:
-----------------------------------------------------------------

This is not really much of a review as this will be off when it lands and getting the testing soon is very valuable. We should review this more once it is in place. We should be able to come kind of consensus about the pooling.

::: gfx/layers/client/SimpleTextureClientPool.cpp
@@ +31,5 @@
> +/* static */ void
> +SimpleTextureClientPool::RecycleCallback(TextureClient* aClient, void* aClosure)
> +{
> +  SimpleTextureClientPool* pool =
> +    reinterpret_cast<SimpleTextureClientPool*>(aClosure);

static_cast<SimpleTextureClientPool*>

::: gfx/layers/client/SimpleTextureClientPool.h
@@ +76,5 @@
> +  //     if we see that, then we should use push_back() to add new elements
> +  //   when we shrink this list, we use pop(), but should use pop_back() to
> +  //   nuke the oldest.
> +  // We may need to switch to a std::deque
> +  std::stack<RefPtr<TextureClient> > mTextureClients;

mAvailableTextureClients

@@ +77,5 @@
> +  //   when we shrink this list, we use pop(), but should use pop_back() to
> +  //   nuke the oldest.
> +  // We may need to switch to a std::deque
> +  std::stack<RefPtr<TextureClient> > mTextureClients;
> +  std::list<RefPtr<TextureClient> > mAutoRecycle;

mOutstandingTextureClients

::: gfx/layers/client/SimpleTiledContentClient.cpp
@@ +81,5 @@
> +  bool fullPaint = false;
> +
> +  RefPtr<TextureClient> textureClient = mManager->GetSimpleTileTexturePool(tileFormat)->GetTextureClientWithAutoRecycle();
> +  //RefPtr<TextureClient> textureClient = mManager->GetTexturePool(tileFormat)->GetTextureClient();
> +  //mManager->GetTexturePool(tileFormat)->AutoRecycle(textureClient);

Comments slipped in
Attachment #8388660 - Flags: review+
Attached patch review comments (obsolete) — Splinter Review
Please merge with tiling and flag as obsolete.
Attachment #8388753 - Flags: review?(vladimir)
Comment on attachment 8388753 [details] [diff] [review]
review comments

got it merged in, thanks
Attachment #8388753 - Flags: review?(vladimir) → review+
Attachment #8388753 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/73d02445dc1c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #9)
> Android R2 failure:
> 
> border-image/border-image-element.html | image comparison (==), max
> difference: 125, number of differing pixels: 5808
> 
> This test compares   { border-image: -moz-element:(#src); } <img id="src"
> src="diamonds.png">   and { border-image: url("diamonds.png"); }.   The one
> using -moz-element seems to be using NEAREST when scaled up, and the second
> seems to be using LINEAR.  There is maybe relevant code:
> 
> http://dxr.mozilla.org/mozilla-central/source/gfx/layers/opengl/
> CompositorOGL.cpp#1117
> 
> Or something else that isn't setting LINEAR properly..

I haven't been able to reproduce this test locally on my GN
Ruling out difference in doBufferedDrawing path (It doesn't use the CreateOffscreen call like euro tiles)
https://tbpl.mozilla.org/?tree=Try&rev=291fe9a234ab
Depends on: 982821
The background-image-element.html test passes if you comment out the filter change here
http://dxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxDrawable.cpp#156
I'm not sure why.
Turns out the background-image-element.html was already mostly failing the same way before and after. Adding one pixel of fuzz makes it go away.
Weird. I don't really like that that test has that much fuzz. Makes it not really useful as a reftest, IMO.
Depends on: 984490
Depends on: 988066
Depends on: 987219
Depends on: 987938
No longer depends on: 987938
No longer depends on: 987219
Attachment #8388649 - Flags: review?(jmuizelaar)
Attachment #8388660 - Flags: review?(jmuizelaar)
You need to log in before you can comment on or make changes to this bug.