Bug 51360 - GNOME notification icons drawn all-black
GNOME notification icons drawn all-black
Status: NEW
Product: xorg
Classification: Unclassified
Component: Driver/geode
unspecified
Other All
: medium normal
Assigned To: X.org Geode Mailing List
X.org Geode Mailing List
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-23 09:05 UTC by Daniel Drake
Modified: 2015-01-10 18:10 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
test app (1.70 KB, text/plain)
2012-06-23 09:05 UTC, Daniel Drake
no flags Details
Hack to try to fix it at the correct place without fallbacks (1.12 KB, patch)
2012-08-07 01:52 UTC, Mart Raudsepp
no flags Details | Splinter Review
screenshot (2.27 KB, image/png)
2012-08-07 21:01 UTC, Daniel Drake
no flags Details
Better hack to try to fix negative srcX/Y cases without fallbacks (3.68 KB, patch)
2012-09-18 05:16 UTC, Mart Raudsepp
no flags Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Drake 2012-06-23 09:05:44 UTC
Created attachment 63374 [details]
test app

After upgrading OLPC XO-1 from Fedora 14 (xorg-server-1.9) to Fedora 17 (xorg-server-1.12) we are seeing problems with GNOME's notification area. The icons appear all-black, which is the same colour as the GNOME panel which they are drawn upon.

I've narrowed this down to a simple test case (attached). This program draws an icon (works OK), then when the window is resized the icon is replaced with all-black.

Initial diagnosis at http://dev.laptop.org/ticket/11860 suggests that we're receiving a composite PictOpSrc request with no mask, repeats or transformations.
lx_do_composite then reacts in this way:

	    /* All black out of the source */
	    if (!exaScratch.repeat && (exaScratch.type == COMP_TYPE_ONEPASS)) {
		    lx_composite_all_black(srcOffset, exaScratch.srcWidth,
			exaScratch.srcHeight);
	    }

and the icon goes black.

We are not 100% sure if this is a Geode bug or something wrong in the X server. A good first step would be to understand how the driver should respond to PictOpSrc with no mask, repeats or transformations, and if that is even valid at all.
Comment 2 Mart Raudsepp 2012-08-07 01:52:09 UTC
Created attachment 65214 [details] [review]
Hack to try to fix it at the correct place without fallbacks
Comment 3 Daniel Drake 2012-08-07 21:01:19 UTC
Created attachment 65250 [details]
screenshot

This doesn't fix the notification area issue. It also makes the test case behave a bit oddly. Here's a screenshot of the window.
Comment 4 Daniel Drake 2012-08-07 21:15:59 UTC
Also, the patch seems to cause further misbehaviour. I launch the test app from a gnome terminal, then I drag the terminal over the top of the test app's window, then away again.

The test app window goes black, and also the titlebar of the terminal window goes black.
Comment 5 Mart Raudsepp 2012-09-18 05:16:13 UTC
Created attachment 67311 [details] [review]
Better hack to try to fix negative srcX/Y cases without fallbacks

A bigger refactoring of lx_do_composite is getting too time consuming (with more candle-lit dates with Porter and Duff in order), so here's again a more direct approach try to fix it for now, as we really should just get a new release out rather quickly now with xserver-1.3 compat and misrenderings of modern desktop fixed.

The previous hack didn't work for GNOME-3 fallback notification icons due to them being actually srcX=0, srcY=-1 or vice-versa, so it hit the unsigned vs signed bug and entered that conditional branch instead of where the negative coordinate handling code was added.

Due to the comment in the patch, this is definitely not final and likely to cause other issues, but might be interesting to test and validate on XO-1 while I figure out and address the potential problem.
Comment 6 Daniel Drake 2012-09-18 21:34:41 UTC
Running that patch, my test case is solved, and the battery icon appears in GNOME's tray. I can't see any new rendering problems. I agree with your approach of releasing this sooner rather than later - thanks for not forgetting about this!

On a similar topic, the network manager icon in the tray is now appearing all-black. But that was the same before applying this patch. I'll investigate and file another bug if it seems to be something geode-related.
Comment 7 Daniel Drake 2012-10-05 14:51:59 UTC
While working on a similar bug in the OLPC's via chrome driver I found a link which explains the magic behind compositing for negative source coordinates:

http://lists.x.org/archives/xorg-driver-geode/2010-June/000688.html

(just in case that info wasn't fresh in your mind - at least it answers my big question of "what does a negative source coordinate mean?")
Comment 8 Mart Raudsepp 2012-11-20 12:21:51 UTC
The exiting out of the loop approach would defeat the purpose of a lot of the changes made by Frank in that area, in particular handling of destination drawing area being larger than the source pixmap for PictOpSrc cases (but not negative srcX/Y, just larger drawing width/height than source pixmap dimensions).
So after some brewing I pushed a different variable tweaking commit that should just make it so that only the source pixmap is composited in this specific case that was completely mishandled before (negative srcX/Y), which should even be correct for PictOpOver, but not so much for PictOpSrc - but I believe not much cases in the wild should need that black rendering for borders, as that's quite undesired to have random black borders in real applications I bet. In the icon case, I believe the rendering is actually to some different pixmap and the black part isn't shown on screen.
A more extensive and full fixing is in order in the future and I have a good idea what to do there, but such changes wouldn't be suitable for an impending stable bugfix release, so hopefully in a major version later on.

Please test the version pushed to GIT if possible, and see that it fixes the icon bug and doesn't cause any regressions elsewhere.
Comment 9 Daniel Drake 2012-11-20 15:46:45 UTC
Thanks a lot! I've tested it briefly - seems to fix the issue, and can't see any other problems. I'll add it to OLPC builds for further testing - if you don't hear back you can assume things are working fine.
Comment 10 Martin-Éric Racine 2015-01-10 18:10:50 UTC
Daniel, is this still an issue with the 2.11.16 driver?