RE: [PATCH] Fix glReadPixels call from read_from_framebuffer (re-redux)

[Stefan Dösinger]

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 :-/

[PATCH] Fix glReadPixels call from read_from_framebuffer (re-redux)

[Nick Burns]

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: [e-mail]
> To: [e-mail]
> 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 :-/

[PATCH] Fix glReadPixels call from read_from_framebuffer (re-redux)

[Stefan Dösinger]

> 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.

[PATCH] Fix glReadPixels call from read_from_framebuffer (re-redux)

[Nick Burns]

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: [e-mail]
> To: [e-mail]; [e-mail]
> 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.

[PATCH] Fix glReadPixels call from read_from_framebuffer (re-redux)

[Stefan Dösinger]

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:[e-mail]]
> Sent: Thursday, December 25, 2008 1:06 AM
> To: [e-mail]; [e-mail]
> 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: [e-mail]
> > To: [e-mail]; [e-mail]
> > 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.

[PATCH] Fix glReadPixels call from read_from_framebuffer (re-redux)

[Nick Burns]

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: [e-mail]> To: [e-mail]; [e-mail]> 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: [e-mail]>> To: [e-mail]>> 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 :-/

[PATCH] Fix glReadPixels call from read_from_framebuffer (re-redux)

[Roderick Colenbrander]

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: [e-mail]> To:
> [e-mail]; [e-mail]> 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: [e-mail]>> To: [e-mail]>> 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 :-/