RE: [PATCH] Fix glReadPixels call from read_from_framebuffer (re-redux)
This patch looks good. There's one last thing we should check: It seems that this is the only code that uses GL_PACK_ROW_LENGTH and friends, so the backup and restore is probably not needed. I think for now it is better to add it because I suspect the code in surface_download_data most likely depends on the default settings without properly controlling them. There's some related driver bug on OSX too(no radar filed yet, unfortunately). Using a PBO for glDrawPixels with a negative pixelzoom(wine uses -1 for y) breaks at least on my radeon X1600 with MacOS 10.5.5. I haven't yet tested it with 10.5.6, but if it is still broken there I have to remember to file a bug. It is sort of a follow-up bug to a bug fixed in 10.5.5; Before that glPixelZoom and PixelPos were completely ignored with PBOs. This bug was on my todo list for a long time by the way. I wanted to fix it, got distracted and forgot again :-/
Thanks for reviewing my patch (it sure makes the SHOGO menu much nicer) BTW do you know if I need to resubmit my other SHOGO patch ([PATCH] Fix ddraw surface version setting)? Concerning negative pixelzoom and drawpixels on R500 Please file a radar on that (and email the mac-opengl mailing list) - Nick ----------------------------------------
From: stefan(a)codeweavers.com To: wine-devel(a)winehq.org Subject: RE: [PATCH] Fix glReadPixels call from read_from_framebuffer (re-redux) Date: Tue, 23 Dec 2008 13:30:40 +0100
This patch looks good.
There's one last thing we should check: It seems that this is the only code that uses GL_PACK_ROW_LENGTH and friends, so the backup and restore is probably not needed. I think for now it is better to add it because I suspect the code in surface_download_data most likely depends on the default settings without properly controlling them.
There's some related driver bug on OSX too(no radar filed yet, unfortunately). Using a PBO for glDrawPixels with a negative pixelzoom(wine uses -1 for y) breaks at least on my radeon X1600 with MacOS 10.5.5. I haven't yet tested it with 10.5.6, but if it is still broken there I have to remember to file a bug. It is sort of a follow-up bug to a bug fixed in 10.5.5; Before that glPixelZoom and PixelPos were completely ignored with PBOs.
This bug was on my todo list for a long time by the way. I wanted to fix it, got distracted and forgot again :-/
Thanks for reviewing my patch (it sure makes the SHOGO menu much nicer) BTW do you know if I need to resubmit my other SHOGO patch ([PATCH] Fix ddraw surface version setting)? Yes, I recommend to do that. It is likely that it was lost or didn't apply any longer after the 3 days it waited in moderation.
You can see on source.winehq.org/git or with 'git log origin' if your patch was applied. If it wasn't, it is best to send a mail to wine-devel asking why, or ask Alexandre(nick 'julliard') on #winehackers. Quite often Alexandre sends a reply if he doesn't like the patch(or someone else does), but not always. Alexandre usually wants a confirmation from someone who knows the area(in the case of d3d Heri, Roderick or me) if a new contributor sends a patch, hence my "the patch is ok" mails. I don't know if Alexandre will apply patches over the holidays, so if the patch is silently ignored it is best to resend it in a few days.
Thanks for the explanation -- I will email asking why soon -- and perhaps resubmit the other patch I have been looking at http://winehq.org/pipermail/wine-cvs to see what has been applied But source.winehq.org/git shows the diffs much nicer - Nick ----------------------------------------
From: stefan(a)codeweavers.com To: adger44(a)hotmail.com; wine-devel(a)winehq.org Subject: RE: [PATCH] Fix glReadPixels call from read_from_framebuffer (re-redux) Date: Wed, 24 Dec 2008 15:48:24 +0100
Thanks for reviewing my patch (it sure makes the SHOGO menu much nicer) BTW do you know if I need to resubmit my other SHOGO patch ([PATCH] Fix ddraw surface version setting)? Yes, I recommend to do that. It is likely that it was lost or didn't apply any longer after the 3 days it waited in moderation.
You can see on source.winehq.org/git or with 'git log origin' if your patch was applied. If it wasn't, it is best to send a mail to wine-devel asking why, or ask Alexandre(nick 'julliard') on #winehackers. Quite often Alexandre sends a reply if he doesn't like the patch(or someone else does), but not always.
Alexandre usually wants a confirmation from someone who knows the area(in the case of d3d Heri, Roderick or me) if a new contributor sends a patch, hence my "the patch is ok" mails.
I don't know if Alexandre will apply patches over the holidays, so if the patch is silently ignored it is best to resend it in a few days.
Your wined3d_gl.h patch was applied, the others weren't Sometimes too much discussion makes AJ wait on a patch too, to see how the discussion turns out. If you resend your patches I'll just reply with an "patch is ok" mail to avoid further confusion
-----Original Message----- From: Nick Burns [mailto:adger44(a)hotmail.com] Sent: Thursday, December 25, 2008 1:06 AM To: stefan(a)codeweavers.com; wine-devel(a)winehq.org Subject: RE: [PATCH] Fix glReadPixels call from read_from_framebuffer (re-redux)
Thanks for the explanation -- I will email asking why soon -- and perhaps resubmit the other patch I have been looking at http://winehq.org/pipermail/wine-cvs to see what has been applied
But source.winehq.org/git shows the diffs much nicer
- Nick
----------------------------------------
From: stefan(a)codeweavers.com To: adger44(a)hotmail.com; wine-devel(a)winehq.org Subject: RE: [PATCH] Fix glReadPixels call from read_from_framebuffer (re-redux) Date: Wed, 24 Dec 2008 15:48:24 +0100
Thanks for reviewing my patch (it sure makes the SHOGO menu much nicer) BTW do you know if I need to resubmit my other SHOGO patch ([PATCH] Fix ddraw surface version setting)? Yes, I recommend to do that. It is likely that it was lost or didn't apply any longer after the 3 days it waited in moderation.
You can see on source.winehq.org/git or with 'git log origin' if your patch was applied. If it wasn't, it is best to send a mail to wine-devel asking why, or ask Alexandre(nick 'julliard') on #winehackers. Quite often Alexandre sends a reply if he doesn't like the patch(or someone else does), but not always.
Alexandre usually wants a confirmation from someone who knows the area(in the case of d3d Heri, Roderick or me) if a new contributor sends a patch, hence my "the patch is ok" mails.
I don't know if Alexandre will apply patches over the holidays, so if the patch is silently ignored it is best to resend it in a few days.
Why not create a texture and draw a quad instead of using glDrawPixels (as it is deprecated in gl3)?Reference -- ogl 3 spec -- (http://www.opengl.org/registry/doc/glspec30.20080811.pdf)Under "E.1 Profiles and Deprecated Features of OpenGL 3.0""Pixel drawing - DrawPixels and PixelZoom (section 3.7.4). However, the language describing pixel rectangles in section 3.7 is retained as it is required for TexImage* and ReadPixels. " - Nick> From: adger44(a)hotmail.com> To: stefan(a)codeweavers.com; wine-devel(a)winehq.org> Subject: RE: [PATCH] Fix glReadPixels call from read_from_framebuffer (re-redux)> Date: Tue, 23 Dec 2008 11:11:08 -0800> > > Thanks for reviewing my patch (it sure makes the SHOGO menu much nicer)> BTW do you know if I need to resubmit my other SHOGO patch ([PATCH] Fix ddraw surface version setting)?> > > Concerning negative pixelzoom and drawpixels on R500> Please file a radar on that (and email the mac-opengl mailing list)> > > - Nick> > ---------------------------------------->> From: stefan(a)codeweavers.com>> To: wine-devel(a)winehq.org>> Subject: RE: [PATCH] Fix glReadPixels call from read_from_framebuffer (re-redux)>> Date: Tue, 23 Dec 2008 13:30:40 +0100>>>> This patch looks good.>>>> There's one last thing we should check: It seems that this is the only code>> that uses GL_PACK_ROW_LENGTH and friends, so the backup and restore is>> probably not needed. I think for now it is better to add it because I>> suspect the code in surface_download_data most likely depends on the default>> settings without properly controlling them.>>>> There's some related driver bug on OSX too(no radar filed yet,>> unfortunately). Using a PBO for glDrawPixels with a negative pixelzoom(wine>> uses -1 for y) breaks at least on my radeon X1600 with MacOS 10.5.5. I>> haven't yet tested it with 10.5.6, but if it is still broken there I have to>> remember to file a bug. It is sort of a follow-up bug to a bug fixed in>> 10.5.5; Before that glPixelZoom and PixelPos were completely ignored with>> PBOs.>>>> This bug was on my todo list for a long time by the way. I wanted to fix it,>> got distracted and forgot again :-/
We have both a glDrawPixels and texture backend. You can switch to a different backend using the RenderTargetLockMode registry key. It has values readdraw (readpixels + drawpixels), readtex (readpixels + texture rendering) and some others (textex and texdraw). I plan on making readtex default at some point. Roderick
Why not create a texture and draw a quad instead of using glDrawPixels (as it is deprecated in gl3)?Reference -- ogl 3 spec -- (http://www.opengl.org/registry/doc/glspec30.20080811.pdf)Under "E.1 Profiles and Deprecated Features of OpenGL 3.0""Pixel drawing - DrawPixels and PixelZoom (section 3.7.4). However, the language describing pixel rectangles in section 3.7 is retained as it is required for TexImage* and ReadPixels. " - Nick> From: adger44(a)hotmail.com> To: stefan(a)codeweavers.com; wine-devel(a)winehq.org> Subject: RE: [PATCH] Fix glReadPixels call from read_from_framebuffer (re-redux)> Date: Tue, 23 Dec 2008 11:11:08 -0800> > > Thanks for reviewing my patch (it sure makes the SHOGO menu much nicer)> BTW do you know if I need to resubmit my other SHOGO patch ([PATCH] Fix ddraw surface version setting)?> > > Concerning negative pixelzoom and drawpixels on R500> Please file a radar on that (and email the mac-opengl mailing list)> > > - Nick> > ---------------------------------------->> From: stefan(a)codeweavers.com>> To: wine-devel(a)winehq.org>> Subject: RE: [PATCH] Fix glReadPixels call from read_from_framebuffer (re-redux)>> Date: Tue, 23 Dec 2008 13:30:40 +0100>>>> This patch looks good.>>>> There's one last thing we should check: It seems that this is the only code>> that uses GL_PACK_ROW_LENGTH and friends, so the backup and restore is>> probably not needed. I think for now it is better to add it because I>> suspect the code in surface_download_data most likely depends on the default>> settings without properly controlling them.>>>> There's some related driver bug on OSX too(no radar filed yet,>> unfortunately). Using a PBO for glDrawPixels with a negative pixelzoom(wine>> uses -1 for y) breaks at least on my radeon X1600 with MacOS 10.5.5. I>> haven't yet tested it with 10.5.6, but if it is still broken there I have to>> remember to file a bug. It is sort of a follow-up bug to a bug fixed in>> 10.5.5; Before that glPixelZoom and PixelPos were completely ignored with>> PBOs.>>>> This bug was on my todo list for a long time by the way. I wanted to fix it,>> got distracted and forgot again :-/
-- Sensationsangebot verlängert: GMX FreeDSL - Telefonanschluss + DSL für nur 16,37 Euro/mtl.!* http://dsl.gmx.de/?ac=OM.AD.PD003K1308T4569a
participants (3)
-
Nick Burns -
Roderick Colenbrander -
Stefan Dösinger