image corruption on Windows CE on youtube.com

RESOLVED FIXED

Status

()

Core
Graphics
P2
major
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: skempe, Assigned: vlad)

Tracking

({verified1.9.2})

unspecified
ARM
Windows CE
verified1.9.2
Points:
---
Bug Flags:
blocking1.9.2 +

Firefox Tracking Flags

(status1.9.2 beta2-fixed)

Details

(Whiteboard: [nv], URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

9 years ago
User-Agent:       Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1; .NET CLR 1.1.4322; .NET CLR 2.0.50727; .NET CLR 3.0.04506.648; .NET CLR 3.5.21022; .NET CLR 3.0.4506.2152; .NET CLR 3.5.30729; Zune 2.0)
Build Identifier: firefox 3.6 for wince6 

The new tab icon shows corruption when brought in focus. Also courrption is observed with youtube's search and upload buttons

Reproducible: Always

Steps to Reproduce:
1.install firefox from http://people.mozilla.com/~vladimir/ce 18th June version
2.check the Open a new tab icon
3.also open youtube.com and check the search and upload buttons 
Actual Results:  
1. the open new tab icon shows corruption
2. youtube search & upload buttons shows horizontal strips. There strips were not observed with older firefox builds.


Expected Results:  
The buttons should not show corruption
(Reporter)

Comment 1

9 years ago
Created attachment 384874 [details]
firefox_corruption.JPG shows the corruption issues
(Reporter)

Updated

9 years ago
Severity: normal → major
OS: Other → Windows CE
Hardware: Other → ARM
I've seen this; it seemed to have started when we turned on OpenGL compositing.  I'll take a look.
Status: UNCONFIRMED → NEW
Component: General → GFX: Thebes
Ever confirmed: true
Product: Firefox → Core
QA Contact: general → thebes
Whiteboard: [nv]
Assignee: nobody → vladimir
Priority: -- → P1

Comment 3

9 years ago
Affects the ListAll tabs icon on the far right as well.
I'm guessing that we're running into some texture size limits here.  When this is seen on youtube, all of youtube's images and backgrounds are in a 169x2800 image.  With the OGL/ddraw backend, we get CLAMP_TO_EDGE behaviour starting at around y=750 or so (you can see this just by opening something like http://s.ytimg.com/yt/img/master-vfl117997.png ).

I'm guessing we're running into texture/surface size limits here; we never actually check the max texture dimensions anywhere.  So the initial fix for this needs to just add those checks, falling back to software surfaces if necessary; then a future patch needs to accelerate rendering from image surface sources.
Created attachment 400548 [details] [diff] [review]
Fallback if the texture size is too large

Add a check for surface extents to _cairo_ddraw_ogl_analyze_pattern. This causes us to fallback for textures that are larger than 2048. This gives correct results, but probably causes a performance impact for large images.
Attachment #400548 - Flags: review?(vladimir)
Comment on attachment 400548 [details] [diff] [review]
Fallback if the texture size is too large

MAX_TEXTURE_SIZE should be queried from GL, not hardcoded.

Also, here's a question -- for the new tab button, the images in question that I see are all on the order of 32/64 pixels in either dimension.  Why do we still see corruption?  Where's whatever big image is in play coming from?
That's a good point, and actually I have a different theory: we're running out of texture RAM, but limiting images to 2kx2k mitigates that by not storing large images, which are more likely to contribute to VRAM exhaustion.

To fix the out-of-texture-memory problem, you have to test out glTexImage2D with GL_PROXY_TEXTURE_2D, then check glGetTexLevelParameter to see if the width is zero. Unfortunately we're not using the usual texture methods, and GL_PROXY_TEXTURE_2D isn't supported on glFramebufferTexture2D or glEGLImageTargetTexture2DOES. :(
That could be, but if we're running out of texture memory, then we wouldn't have anything drawn (or black, I forget what the state is for an incomplete texture).  There's a way to do that check with framebuffers, I can't remember what it is though.. would have to read the code.
Created attachment 400859 [details] [diff] [review]
Fallback if the texture size is too large v2

This version checks the maximum texture size at runtime.

I also did some more testing and this only fixes the youtube problem (large image) not the new tab one. I'm not sure what's happening with the new tab one.
Attachment #400548 - Attachment is obsolete: true
Attachment #400859 - Flags: review?(vladimir)
Attachment #400548 - Flags: review?(vladimir)
The tab button is interesting. Removing the '+' image gets rid of most of the corruption, however, there's still some corruption in between the tabs and the button. It also seems like we might be repainting all the chrome when the throbber throbs, because the corruption changes as while we are throbbing.

The '+' image is done using -moz-image-region which makes me suspicious that it is '-moz-image-region' that's causing the problem, but I can't think of why...
Attachment #400859 - Flags: review?(vladimir) → review+
Duplicate of this bug: 511653
It looks like the new tab part of the bug is caused by improper locking of the surface.

The corruption happens when we are drawing three surfaces:

1. comp: over, src: 0012E620 dst: 020341A0, (25,24)
2. comp: over, src: 0012E620 dst: 020341A0, (25,24)
   composite of non ddraw surface 25,24
3. comp: over, src: 0012E828 dst: 020341A0, (18,18)

The second operation falls back to software because we are painting an animated image and those frames are image surfaces instead of ddraw surfaces.

To do the software fallback, we call acquire_dest_image() this locks the surface if it hasn't already been locked. However, we have no release_dest_image() implementation so the surface isn't unlocked afterwards.

I'm not sure what the cache architecture is like on these devices is like but it looks like when we do the third operation with the gpu, it doesn't have a consistent image for it to draw to.

NVidia folks, does this explanation sound reasonable?

Comment 13

9 years ago
The locking/unlocking is lazy...  We don't unlock as part of the release_dest_image(), but we do as part of the next composite call (in _cairo_ddraw_ogl_reference(), called by _cairo_ddraw_surface_bind_to_ogl(), called by _cairo_ddraw_ogl_bind_program()).
(In reply to comment #13)
> The locking/unlocking is lazy...  We don't unlock as part of the
> release_dest_image(), but we do as part of the next composite call (in
> _cairo_ddraw_ogl_reference(), called by _cairo_ddraw_surface_bind_to_ogl(),
> called by _cairo_ddraw_ogl_bind_program()).

_cairo_ddraw_ogl_reference() only seems to call lock() and not unlock()...

Comment 15

9 years ago
Ah correct...  it does a Lock() as a work-around to not having eglWaitNative(), which is what it really wanted to do (to force D3D to finish using the surface before trying to use it with GL).

This works because GL doesn't care about the D3D lock...  the EGLImage sibling is valid whether the D3D lock is held or not...

That being said, the problem may very well be due to caching...  It may be that the fragment data cache is keeping a stale copy of the destination surface and it doesn't pick up the changes that the software fallback makes...  I personally don't know enough about the driver to comment on that...  but it probably does need some kind of GL/EGL call to make sure the cache is invalidated (although glFinish() should have been called as part of the _aquire call)...
Duplicate of this bug: 519597
Summary: corruption observed on the open new tab icon → image corruption on Windows CE
Duplicate of this bug: 520548
Duplicate of this bug: 517505
What's the status here? Does the attached fallback patch need checked in, or is this waiting for an NVidia driver bugfix?
Flags: blocking1.9.2+
Priority: P1 → P2
The max texture size is checked in:
http://hg.mozilla.org/mozilla-central/rev/a90ee28cac5f
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Comment on attachment 400859 [details] [diff] [review]
Fallback if the texture size is too large v2

I assume this is safe for 1.9.2, where we'll need it?
Attachment #400859 - Flags: approval1.9.2?
Comment on attachment 400859 [details] [diff] [review]
Fallback if the texture size is too large v2

Bah, this is a blocker no no approval needed.
Attachment #400859 - Flags: approval1.9.2?
Verified fixed on ther 1.9.2 branch using  Mozilla/5.0 (Windows; U; WindowsCE 6.0; en-US; rv:1.9.2b2pre) Gecko/20091104 Namoroka/3.6b2pre. I no longer see the image corruption on youtube or the tab button.
Keywords: verified1.9.2
Suyog sent me an email seeing they were still seeing the tab corruption using B2 build - I asked them for a screen shot.
Using the latest nightly, 20091112, I do see the corruption on tab mouseover. So I am reopening this bug for further investigation.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Yes, as jeff said in previous comments, there are two different bugs here; this bug ended up being specifically for the youtube issue.  Jeff or Marcia, can you make sure we have a bug on file specifically for the tab issue?
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
(In reply to comment #27)
> Yes, as jeff said in previous comments, there are two different bugs here; this
> bug ended up being specifically for the youtube issue.  Jeff or Marcia, can you
> make sure we have a bug on file specifically for the tab issue?

We do. Bug 524217.
Ok, I missed that one since it did not have [nv] in the whiteboard. I just added it.
Summary: image corruption on Windows CE → image corruption on Windows CE on youtube.com
You need to log in before you can comment on or make changes to this bug.