Incorrect repeat rendering on XO-1.5

[dsd]

Running this test app shows a problem with repeated rendering:

https://bugs.freedesktop.org/attachment.cgi?id=63374

The image shown there is displayed in the center of the window; the window automatically resizes after a few seconds, and is then rendered many times (it should only be rendered once).

I imagine fixing this will fix a real issue we have in 13.1.0: the battery icon in the GNOME notification area is partially repeated in the space to the left and right of it.

I imagine this is a bug in the EXA codepaths exposed by new cairo/gtk/something.

[dsd]

Disabling composite works around the issue (confirming that the issue is in composite code).

The test app generates a single composite request: PictOpSrc with these parameters:

Composite src: -38,-38 mask: 0,0 dst: 0,0 width: 100, height: 100

[dsd]

There is no repeat or transform on the source picture, and there is no mask.

[dsd]

The source picture has dimensions 24x24 and destination 100x100.

If the source is 24x24, how can we composite an image larger than the source itself starting from a coordinate outside of its bounds? This gives a good answer: http://lists.x.org/archives/xorg-driver-geode/2010-June/000688.html

In short, it is valid for EXA to ask us to go out of the bounds of the source picture - including negative coordinates. Because no repeat is specified, all the area outside of the source picture must be interpreted as 0 - black.

When you think about this composite request and what the output rendering should be, it makes sense.

In this case, it looks like the hardware is wrapping out-of-bounds accesses back into the source pixmap, causing the repeat effect. As no repeat is specified, this isn't what we want here.

Is there any documentation for the engine used for this work? e.g. all the magic in via3DEmitQuad_H5 ? Maybe we can turn off the coordinate wrapping and ask it to provide us with black.

If not, I see a few options for how to handle this, once we have detected the out-of-bounds case:

# Perform an all-black solid fill of the entire destination region first, then perform a clipped composite OpSrc operation to put the source picture in the right place (without going outside of its coordinate bounds) # Perform an appropriately clipped OpSrc composite operation to put the source picture in place, then do 4 solid fills around the outside of it to make the rest of the region black. # Perform an appropriately clipped OpSrc composite operation to put the source picture in place, then construct a mask that fills the destination region but excludes the final location of the source picture, then perform a solid black fill using the mask.

Which of these options do you see as most realistic?

[jnettlet]

I have a simpler proposition. Have you verified we are not hitting a bug where the v3d src repeat state is not getting set to TRUE when it should not be? We track that value from the PrepareComposite hook and use it to setup the texture attributes which are then processed and sent off to the 3d hardware.

[dsd]

The render spec also explains clearly what Repeat=None means w.r.t. out-of-bounds data access:

9. Source and Mask Transformations

When fetching pixels from the source or mask pictures, Render provides four
options for pixel values which fall outside the drawable (this includes
pixels within a window geometry obscured by other windows). 

 +	None.  Missing values are replaced with transparent.

So I guess when I wrote "black" above I really meant "transparent".

Yes, I have checked that the repeat flag is not being set.

I got lucky and stumbled across a fix here; the ViaTextureModes enum values control what the hardware does w.r.t. accessing out-of-bounds data.

Patch attached - fixes the problem, and can't see any regressions after a quick spin in GNOME and Sugar.

[dsd]

fix

[jnettlet]

Great work. The transparent makes more sense. When you originally said black I was thinking why this was being sent down the Composite pipeline instead of being a Solid Fill/Clear and a Blit.

I just went through some change logs and it appears that Cairo is using more of the "Render" OpenGL aspects. As you already figured out Texture Mode S/T is GL lingo.

It used to be common practice that we only ever handled > RepeatTypeNormal, and anything else would get punted to software. Then if the operation was < Normal then Cairo would never provide negative coordinates. I guess that got fixed to optimize another graphics driver somewhere down the road.

You should probably send this fix over to the openchrome guys. I believe they have the same bug in their codebase.

I will test and commit this and provide new rpms.

[dsd]

Thanks. It does look like the same bug might be hit there, I'll write them a mail once the fix is pushed to git.

[dsd]

Fixed in xorg-x11-drv-chrome-5.74.33-13 and sent a mail to the openchrome list.

Test case: in GNOME, look at the battery in the system tray. It should be rendered cleanly with no junk around the edges.

[dsd]

Test in 13.1.0 build 9.

[greenfeld]

The GNOME battery icon looks correct on XO-1.5 in 13.1.0 os26.