The rendering bug of geode driver has been reported on several BTS on various linux distribution. You can see the unwanted green or other color will be displayed on the gtk-applet, such as that on File Browser. At the same time, from the bug 15700, a lot of bug (progressbar and scrollbar) on rendering is reported there. I am now checking this bug on the lx_do_composite() function in geode driver. The lastest founding is that. For the SRC and OVER operation on the rendering, the driver will wrongly handle the parameters which is given by client application. I have given a modification on SRC operation. Right now I am focused on OVER operation. when our driver handles the Over operation where source image has the alpha value and mask has the alpha value, our driver does the wrong thing. It uses the mask's alpha value to do the composite operation and omit the previous alpha value. The correct formula is that the source(ARGB) should be multiplied with the mask alpha value, then use the new alpha to do the OVER operation. I have done many experiments to prove that and guys from xorg-devel mail groug gives me the hint. In short, our driver is short of one pass of Source*(alpha of mask) and wrongly use a*A+(1-a)*B to do the OVER operation. See datasheet of Geode on GP_RASTER_MODE for detail. I'll correct this huge bug and let you know my result. Thanks, Frank
Make progress in rendering bug in OVER. See my attached pictures for progress bar on new driver. Martin and Mart, give the patch on xorg-geode mail group?? Thanks, Frank
Created attachment 36027 [details] This is old geode driver drawing the progressbar and its result This is old geode driver drawing the progressbar and its result
Created attachment 36028 [details] This is new geode driver drawing the progressbar and its result This is new geode driver drawing the progressbar and its result
Created attachment 36029 [details] correct result on ATI graphics card as a comparison correct result on ATI graphics card as a comparison
Created attachment 36030 [details] This is new geode driver drawing the scrollbar and its result This is new geode driver drawing the scrollbar and its result
(In reply to comment #1) > Make progress in rendering bug in OVER. See my attached pictures for progress > bar on new driver. > > Martin and Mart, > give the patch on xorg-geode mail group?? Preferably attach the patch to this bug.
Created attachment 36056 [details] [review] The new patch for the rendering The new patch for the rendering
I have put the patch here and need Martin's help to commit. Hope you guys give me more feedback on the patch. Any questions are welcome. I'll go on researching. Thanks, Frank
Created attachment 36092 [details] [review] Patch to solve the rendering issue on 2.11.8 version
1.Change the srcColor type from unsigned int to unsigned long for 32 bit color value use(for example. ARGB32) 2.Add the maskrepeat to record if the mask picture is repeatable 3.Correct the PictOpOver operation in lx_alpha_ops struct 4.Return FALSE in lx_check_composite() for every FALSE condition in lx_prepare_composite(), forbidding the pixmap migration as soon as possible 5.For PictOpOver operation, get the source color from the source pixmap. If that is RGB565, convert is to ARGB32; If that is ARGB32, get the first pixel color 6.Correct the PictOpSrc and PictOpOver operation with mask value 7.Add a fucntion lx_do_composite_maks_opover() to handle the PictOpOver condition with source ARGB32 and maks A8 8.Fix the progressbar and scrollbar bug Thanks, Frank
Ideally these change would be worked on in GIT with GIT capabilites, so you can easily make it be 8 patches, instead of one big. So one patch per change. This way it would be much easier to review, get things right, and that's just the right way in open source and GIT :) I hope the changes can be split apart into multiple commits/patches from hereon. Please contact me for any help needed with that. I tested the patch from Friday, which seems similar or the same, and that indeed nicely fixed up my scrollbar/button borders and progressbar drawing issues, but didn't look closely on the concrete code changes yet. Comments on things described in the changelog: 1. Why do you need to change unsigned int to unsigned long to store 32bit values? Unsigned int and unsigned long are both 32bit on x86, and conceptually unsigned int is more correct (also "unsigned long" is 64bit on x86-64, but geode is 32bit of course). Even more correct would be something like __u32 (a Xorg typedef to uint32_t), but we should use whatever other places use for storing 32bit colors, but that's just choice of which typedef. 4. If we can reliably catch inability to accelerate already in CheckComposite (all information in the checks is available to use at CheckComposite time), then we should indeed do it in there to fallback as early as possible (before pixmaps are migrated to GPU memory - memcpy overhead). But this also should mean, that we don't need to do the checks in PrepareComposite anymore, as if CheckComposite returned FALSE, we are guaranteed to not get a call to PrepareComposite with the same rendering commands. So the checks that can be done earlier should moved, not copied. All in all, seems like awesome work, as it does fix most of my most awful rendering problems :) Now lets get this in properly in multiple commits and so on, so it would be easier to do it like that in the future too :)
Created attachment 36207 [details] Picture in chaos in file browser
(In reply to comment #12) > Created an attachment (id=36207) [details] > Picture in chaos in file browser Did you obtain this screenshot with or without your patch applied?
This condition can be obtained without my patch. Still I suspect it is a PictOpOver opearation bug. I am finding it now. If you have any good simple example that can reproduce the same issue, please attach your application here. Thanks, Frank
Answer for Mart's reply,sorry for late. Split it into 7 patched commited in xorg-driver-geode. I have acceptted your two suggestion below in my patch. Thanks, Frank
Created attachment 36229 [details] Attach a gtk program to reproduce this issue
Created attachment 36230 [details] Attach this gtk program's result with rendering error(see green part)
Add a program attachment to reproduce the chaos picture issue.
(In reply to comment #9) > Created an attachment (id=36092) [details] > Patch to solve the rendering issue on 2.11.8 version I tested this patch on the XO-1. It fixes the black boxes in Gnome and works well across all the Sugar activities I could test.
Would now be a good time to push Frank's 7 patches into GIT to facilitate wider testing?
(In reply to comment #20) > Would now be a good time to push Frank's 7 patches into GIT to facilitate wider > testing? I have not done any reviewing, but I'd like to see this code land in git. Then I'll use a git snapshot to build a package for Fedora 11, which is what currently runs on the XO-1 and XO-1.5.
Bernie, thanks for your test. Glad to hear the news that this patch can work for 15700. I suggest you guys after review, please commit these patches to community for users to test to give me more feedback. Thanks Frank
Give a new patch(the 8th) for your test to solve the chaos picture issue. I should make sure this patch reasonable after I discussed with guys on xorg-devel. Thanks, Frank
Created attachment 36432 [details] Wrong rendering on Firefox characters Wrong rendering on Firefox characters
Created attachment 36486 [details] Black region and discrete line in firefox Black region and discrete line in firefox
Allocate the FB memory from 24MB to 32MB in BIOS, firefox mis-rendering issue is disappearring. Off-screen memory region is not big enough for use in 24MB condition. But the patch's modification is correct:). Reserch more on Off-screen memory allocation under 24MB and its use. Thanks, Frank
Copy from mail: >Have you narrowed down if the problem is due to your code failing to handle an offscreen memory allocation failure, or something else? Your guess is same as me after I enlarge the off-screen memory size that result in better result. Based on my debug, the bug is right here:). Today I have fixed this bug in our driver. Just as the previous code writes, the off-screen memory size is fixed 256KB(which is reserved long long ago), and in firefox, when you press a refresh button, a big mask region 768x268 is need to be rendered with PictOpOver. You know, for the PictOpOver, the first pass is to do src*mask, and the result is ARGB format. So the off-screen size is 768x268x4=823296 > 256KB, so it overflows! The black region and missing glyph character are due to this. After I know this, I adjust the off-screen region to 2MB instead of 256KB. As expected, the firefox mis-rendering bug is missing. That proves my guess and the debug result. But as you know, our Framebuffer size is not big enough in Geode, especially for OPLC computer. And 2MB is not big enough. Take an example, 1280x1024x4 is nearly 5MB. So that is possible to allocate so huge memory for off-screen operation. And Jonathan give me one-line hint, use scanline method(Jonathan usually give me the direction!). So I split the rendering into several chunk, each is 256KB, then do it. Right now, It works fine:). I am happy with this. You know that is very important for us to use a small off-screen region that can make other FB region free. So this patch is valuable. Thanks, Frank
Created attachment 37062 [details] 20x20 source picture, 40x40 rendering region with 4 colours 20x20 source picture, 40x40 rendering region with 4 colours
Created attachment 37063 [details] out of region, src is 22, compared with previous one out of region, src is 22
Created attachment 37290 [details] wrong glyph rendering using HW+SW wrong glyph rendering using HW+SW
Lower the priority of this issue. The patches for misrendering main bug has been released and in review.
Update for patch 4: http://lists.x.org/archives/xorg-driver-geode/2010-August/000952.html Thanks, Frank
5b93fdd42d7d8af4535fd62ce0749f2c4434f9fe in Geode 2.11.9 fixes this if I'm not mistaken?
This issue can be closed now. Main rendering bugs have been solved in 2.11.9 release. The fixed ops including PictOpOver, PictOpSrc, PictOpClear, PictOpIn, PictOpInReverse, PictOpOut, PictOpOutReverse, PictOpAdd. Any new bug reported, we can reopen this issue. Thanks, Frank
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.