[Bug 14762] New: Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762

           Summary: Max Payne 2: sepia tone filter issues (high PPE)
           Product: Wine
           Version: 1.0.0
          Platform: PC-x86-64
               URL: http://www.rockstargames.com/maxpayne2/mp2_downloads.htm
                    l
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: directx-d3d
        AssignedTo: wine-bugs(a)winehq.org
        ReportedBy: liquid.acid(a)gmx.net
                CC: onety-three(a)gmx.net


Hi,

this is a cleaned up version of the issues first described in
http://bugs.winehq.org/show_bug.cgi?id=14038.

Game affected: Max Payne 2 (demo version tested)

Issue: MP2 uses some fullscreen post-processing effects to enhance gaming
experience. The famous bullet time effect is using a sepia tone filter, which
breaks on wine when certain ingame options are set.

MP2 offers two modes of post-processing effect quality (switching them
completly off is also possible):
(i) medium -> the sepia filter works
(ii) high -> sepia filter breaks

Notice that MP2 also uses some other PP (post-processing) effects, like a
motion blur filter in cutscenes. The filter from the cutscenes is NOT affected
by this issue described here, although it also affects the entire screen.

The issue also appears with wine-0.9.56.
Issue is gone when going back to wine-0.9.45, but the rendered area is now
confined in a rectangular portion of the visible screen. (read more about this
at the end (*))

Regression testing turned up this commit:
[d09cbcec91561186bee77aaba55b29cbf2d7e7ef] wined3d: Activate
GL_ARB_texture_rectangle.

http://source.winehq.org/git/wine.git/?a=shortlog;h=d09cbcec91561186bee77aab...
Notice the other five wined3d commits by Stefan Dösinger.

Somehow using ARB_tex_rect instead of using NP2 emulation breaks the sepia
filter, but leaves the motion blur intact.
In fact this commit does also correct "scaling" issues with the other PP
effect, motion blur (again see below (*)).

My wine settings for MP2:
UseGLSL=disabled
OffscreenRenderingMode=backbuffer
RenderTargetLockMode=auto

It doesn't matter though is ORM is set to backbuffer or fbo, the issue also is
reproducible with ORM=fbo. However testing this with ORM=backbuffer makes it
easier for me because of other issues with FBO (plus FBO generates a lot of
FIXME noise on the console).

Graphics hardware used to confirm:
(i) Me, using a Geforce FX 5900
(ii) Frank Roscher, using a Geforce FX 5700

Drivers versions:
(i) FX 5900 driven with 100.14.19 and 173.14.09 (makes no difference)
(ii) FX 5700 driven by driver version 169.12

(*) Interesting facts with wine-0.9.45:
Like I said above PPE set to high work with this version, but the rendered area
is suffering from "scaling issues". This can be reproduced all the time with
the bullettime filter.

However there are also the cutscenes, using the motion blur filter. The funny
thing there is that they are also affected by the scaling issue, BUT not
always.
Watch the first cutscene sequence after you gain control over Max in the
hospital room (the cutscene begins when you are right in front of the double
door). The scaling issue changes during the cutscene to correct rendering, and
back after some moments.
I don't know if the changes from "wrong scaling" to "correct scaling" and
vice-versa are always the same but I'm going to check that again.

Greets,
Tobias


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762


Tobias Jakobi <liquid.acid(a)gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |stefandoesinger(a)gmx.at
  Status Whiteboard|                            |regression, download




-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762


Tobias Jakobi <liquid.acid(a)gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jb.faq(a)gmx.de




-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762


Tobias Jakobi <liquid.acid(a)gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |alexd4(a)inbox.lv




-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762


Tobias Jakobi <liquid.acid(a)gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hverbeet(a)gmail.com




-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #1 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-08-04 15:48:03 ---
Alexander Dorofeyev could verify this bug on his Geforce6100 (integrated) by
commenting out ARB_tex_npot support in directx.c
(see http://bugs.winehq.org/show_bug.cgi?id=14038#c64)


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762


Austin English <austinenglish(a)gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |download, regression
  Status Whiteboard|regression, download        |




-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #2 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-08-04 17:05:55 ---
Jan Bücken can confirm this on his ATI Mobility Radeon HD2600 with latest git
master.
Drivers: ati-drivers-8.512 (=8.7, =8.51.3) (fglrx)

He has also disabled ARB_tex_npot in directx.c


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #3 from Alexander Dorofeyev <alexd4(a)inbox.lv>  2008-08-04 17:35:01 ---
Yes it's there on geforce6100 with non-pow2 extension disabled. In backbuffer
it's "bullet time filled with solid color", in fbo it's even worse in the
current git, the "corridor flythrough" intro with motion blur shows total
corrupted garbage and console is spammed with tons of errors including some
glsl shader stuff (i have it default which is on i guess).

Disabling texture_rectangle as well in directx.c makes it render
semi-correctly, even with orm=fbo, but there's is only a subrectangle rendered
in bullet time.

Either way lots of:

fixme:d3d:transform_texture Non-power2 texture being used with generated
texture coords

I guess what this all means is there were several versions of code none of
which really worked 100% perfectly but each has its own drawbacks. For this
particular game older code used to work better.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762


Jan Buecken <jb.faq(a)gmx.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1




--- Comment #4 from Jan Buecken <jb.faq(a)gmx.de>  2008-08-04 18:28:52 ---
*** This bug has been confirmed by popular vote. ***


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #5 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-08-04 19:03:22 ---
(In reply to comment #3)
> Yes it's there on geforce6100 with non-pow2 extension disabled. In backbuffer
> it's "bullet time filled with solid color", in fbo it's even worse in the
> current git, the "corridor flythrough" intro with motion blur shows total
> corrupted garbage and console is spammed with tons of errors including some
> glsl shader stuff (i have it default which is on i guess).
> OK, I have GLSL disabled globally. But you might wanna take a look at:
> http://bugs.winehq.org/show_bug.cgi?id=14751

I can reconfirm this (nearly) black screen bug with ORM=fbo, but not with
ORM=backbuffer. Maybe that's related.
> Disabling texture_rectangle as well in directx.c makes it render
> semi-correctly, even with orm=fbo, but there's is only a subrectangle rendered
> in bullet time.
> I think we're hitting a safe codepath here.
> Either way lots of:
>
> fixme:d3d:transform_texture Non-power2 texture being used with generated
> texture coords
> Hmm, I only get these with mirrors enabled. But I have to check again. Can you
> try turning mirrors off in the ingame options?
> I guess what this all means is there were several versions of code none of
> which really worked 100% perfectly but each has its own drawbacks. For this
> particular game older code used to work better.
> I hope this is fixable, though :)

-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762


H. Verbeet <hverbeet(a)gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|hverbeet(a)gmail.com          |




-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #6 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-10-07 15:34:26 ---
Reconfirming with wine-1.1.5 and wine git master.

ORM=backbuffer, PPE=high: motion blur works, sepia doesn't
ORM=fbo, PPE=high: nothing works (see bug #14751)


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #7 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-11-02 06:52:10 ---
Note to me: Try dumping GLSL shader source with medium and high PPE settings
and compare them.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #8 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-11-02 12:44:44 ---
Created an attachment (id=17046)
 --> (http://bugs.winehq.org/attachment.cgi?id=17046)
WINEDEBUG=+d3dshader extract

Did two runs with WINEDEBUG=+d3d_shader and switching PP effect to medium /
high.

Grepped both logs for shader_trace_init. D3D asm code with both runs is the
same, so this doesn't seem to be the issue.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #9 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-11-02 12:57:31 ---
I tried to somehow verify what I suspected to be the issue.

Claim: The issue is due to incorrect texture coordinates. Maybe the ones for
the quad/tris when doing the blit of the processed framebuffer image.
It seems like texture coordinates are supplied in the range [0,1]x[0,1] while
the texture object used is of type RECT and therefore wants them in range
[0,width]x[0,height].

This is due to the lack of hw-accelerated arb_tex_npot support (only
arb_tex_rect is supported without software fallback). Wine doesn't seem to
correct fixup the texcoords.

The following three screenshots "verify" the claim.
Explanation:
1) First shot: Notice the light switch in the upper left corner of the screen.
It has a red area inside.

2) Second shot: Positioning the camera so the most upper-left pixel is a redish
one. I dunno how D3D adresses the screen, but this is possibly what ends up
being pixel with adress (0,0) in the framebuffer.

3) Third shot: Activating fb effect (bullettime / sepia filter). This ends up
producing a screen with redish color flood-fill.

Try with other object with distinct colors. It's always the same.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #10 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-11-02 13:00:39 ---
Created an attachment (id=17047)
 --> (http://bugs.winehq.org/attachment.cgi?id=17047)
img seq 1/3 (see comment #9 for explanation)


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #11 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-11-02 13:00:59 ---
Created an attachment (id=17048)
 --> (http://bugs.winehq.org/attachment.cgi?id=17048)
img seq 2/3 (see comment #9 for explanation)


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #12 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-11-02 13:01:17 ---
Created an attachment (id=17049)
 --> (http://bugs.winehq.org/attachment.cgi?id=17049)
img seq 3/3 (see comment #9 for explanation)


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #13 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-11-02 14:03:23 ---
Additional hacking:

I figured out that with my setup the GSLS program used for the PP screen
processing was always no. 12.

Checked the traces and it was revealed that program 12 was composed out of
shader objects 10, 11 and 13.

Object 10 (in PPE=high mode) was reading something from a rect sampler.
Compared this with the code for PPE=medium. Not a rect sampler this time, but a
normal 2d one. This was the only difference in the GLSL program code.

------------------------------------------------------------

Object 10 (PPE=high):
#version 120
#extension GL_ARB_texture_rectangle : enable
uniform vec4 PC[8];
uniform sampler2DRect Psampler0;
uniform sampler2D Psampler2;
vec4 T0 = gl_TexCoord[0];
vec4 T1 = gl_TexCoord[1];
vec4 T2 = gl_TexCoord[2];
vec4 R0;
vec4 tmp0;
vec4 tmp1;
void main() {
T0.xyzw = (texture2DRect(Psampler0, T0.xy).xyzw);
tmp0.x = dot(T1.xyz, T0.xyz);
tmp0.y = dot(T2.xyz, T0.xyz);
T2.xyzw = (texture2D(Psampler2, tmp0.xy).xyzw);
R0.xyzw = (mix(T0.xyzw, T2.xyzw, PC[4].xyzw));
gl_FragColor = R0;
float Fog = clamp(gl_FogFragCoord * gl_Fog.start + gl_Fog.end, 0.0, 1.0);
gl_FragColor.xyz = mix(gl_Fog.color.xyz, gl_FragColor.xyz, Fog);
}

------------------------------------------------------------

For the GLSL source with PPE=medium just replace sampler2DRect with sampler2D
and texture2DRect with texture2D.

My current ingame resolution is 640x480. This crude hack returns proper
rendering of the bullettime effect:

In shader_glsl_generate_pshader (dlls/wined3d/glsl_shader.c) add this before
the shader_generate_main(...) call:
if (shader_obj == 10) {
    shader_addline(buffer, "T0.x = T0.x * 640.0;\n");
    shader_addline(buffer, "T0.y = T0.y * 480.0;\n");
}

You probably have to adjust the 10 to the id that is used on your machine.
640.0 and 480.0 are of course the ingame resolutions.

Greets,
Tobias


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #14 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-11-02 17:09:01 ---
Note to me: Try to understand the functions Henri told me.
Look for functions (in glsl_shader.c) calling shader_glsl_get_sample_function.

List of functions where texrect has an effect:
pshader_glsl_tex
shader_glsl_texldl

Figure out a effective way of passing texture dimensions for fixup into the
GLSL shader.

Static hardcoding would be the best in the case of Max Payne 2 since the
texture dimension only changes when the ingame resolution is switched. This
can't be done while the engine is running. A restart is needed.

However this doesn't look like a good generic way to do this. Need more input
on this one...

Or pass tex dimensions via another texcoord, uniform, whatever. Still don't
know GLSL well enough... :(


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #15 from Stefan Dösinger <stefandoesinger(a)gmx.at>  2008-11-02 17:14:19 ---
You could try to implement a coordinate correction like this, but it is pretty
tricky. Here are a few rules that have to be implemented:

1) No vertex or pixel shader: Correct the coords in the texture matrix
2) vertex shader, but no pixel shader: Correct the coords in the vertex shader
3) Pixel shader, vertex shader yes or no: Correct the coords in the pixel
shader

(1) and (2) are needed because without a pixel shader there is no other option
to fix the coordinates. OK, you may get away with it if a fixed function
pipeline replacement is active, e.g. GL_ATI_fragment_shader, or
GL_ARB_fragment_program, but this would be kinda ugly.

(3) is needed because pixel shaders can do arbitrary calculations in the
texture coords before they sample the texture

Now the ugly part is that this adds many dependencies across different states.
It ties pixel, vertex, texture, texture matrix states all together, although
the are mostly independent right now. The performance implications would be not
so good.

The other bad aspect is that the coordinate correction needs at least 2 single
float uniforms per sampler, which are supposed to be available to the
application. (even hardcoding the values into the shader won't help - the
driver needs a uniform for this)

I think the best is to close this bug as WONTFIX and ignore the issue. It only
occurs on one card type, the GeForce 5 series. All other cards either have
native NP2 texture support, or the driver emulates it in hardware if we stick
to the texture rectangle limitations.

I really hate to do that - my credo is that everything that works on Windows
should work on Wine, with the same hardware, at the same feature level and the
same performance. But this is one of the corner cases we can't fix.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #16 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-11-02 18:20:33 ---
(In reply to comment #15)
> The other bad aspect is that the coordinate correction needs at least 2 single
> float uniforms per sampler, which are supposed to be available to the
> application. (even hardcoding the values into the shader won't help - the
> driver needs a uniform for this)
> I wonder if this has any effect at all on the current situation.
> I mean the fixup code would only be needed for the 5-series. And currently
> situations where it's needed are not even rendered close to what it looks like
> in a native environment. So if after some fixup code exists the amount of
> uniforms is too large for the driver, we'll end up in the same situation. Scene
> is not rendered correctly.

Or would this have additional impacts on previously "working" apps?

I'm just referring to the uniform count. I totally agree that additional
depedencies inside the state code are a really bad thing.
> I think the best is to close this bug as WONTFIX and ignore the issue. It only
> occurs on one card type, the GeForce 5 series. All other cards either have
> native NP2 texture support, or the driver emulates it in hardware if we stick
> to the texture rectangle limitations.
> What's going on with other cards, like the <=4-series? I dunno their HW caps,
> but AFAIK the GeForce2 already support tex_rect, but probably no real tex_npot.
> I realize that these cards are old, but thanks to nouveau we'll probably get
> some long-term support for these :)
> I really hate to do that - my credo is that everything that works on Windows
> should work on Wine, with the same hardware, at the same feature level and the
> same performance. But this is one of the corner cases we can't fix.
> I wonder how this is done in a native environment.

How comes the OpenGL driver implements this software fallback when on Windows
it works just fine. Is this really a flaw in the hardware design or is it just
the driver?

If it's a hw thing then why can't I find any information about a similar
fallback in D3D?

Greets,
Tobias


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #17 from Stefan Dösinger <stefandoesinger(a)gmx.at>  2008-11-02 20:25:54 ---
> I wonder if this has any effect at all on the current situation.
> I mean the fixup code would only be needed for the 5-series. And currently
> situations where it's needed are not even rendered close to what it looks
> like in a native environment.
> You definitely have a point here. It can't be any worse than broken.

Ok, one may argue that it can. Currently the rendering is incorrect. The worst
case with the uniform limitation is that the app fails to run(if we report the
limit), or the shader fails to compile(thus incorrect rendering, gl errors,
etc)
> I'm just referring to the uniform count. I totally agree that additional
> depedencies inside the state code are a really bad thing.
> We can now build the state linking based on the GL extensions that are
> available. You may be able to limit the problems to GF5 cards
> What's going on with other cards, like the <=4-series?
> GF2 cards have no pixel shaders and no vertex shaders. We just fix up in the
> fixed function pipeline and are happy

GF3 and GF4 cards have vertex shader support in wine right now, so in theory we
can run into the VS+FFP problem, but I haven't seen any app doing that yet.
They also support pixel shaders, but only via GL_NV_texture_shader, which we
don't support for pixel shaders yet. If we implement it, we have the same
problem on those cards
> I wonder how this is done in a native environment.
> It works just fine, because in d3d conditional NP2 textures are addressed using
> normalized coords. This is an odd invention of GL_ARB_texture_rectangle that
> makes the problems here.

So here's the possible irony: We burn 2 uniforms per texture to upload the
texture size, and the opengl driver probably re-normalizes the coordinates and
eats another 2 uniforms. So in the end we may need a full 4 component vector to
something that is a NOP in the end.

wrt noveau, if it ever goes somewhere, we can implement the fast GL_TEXTURE_2D
emulation that we make use of with WINE_normalized_texrect to avoid the whole
mess.

You could file a feature request at Nvidia too, it's worth a try.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #18 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-11-24 14:39:02 ---
Just a quick note: Another game is affected by this problem.
FEAR 1.08 doesn't correctly render it's "bullettime mode". Also some effects
are missing, like flames (attaching screenshot).

I'm resolving my other bug (http://bugs.winehq.org/show_bug.cgi?id=15875) about
HL2 to duplicate, because it's without doubt the same problem.

I'm not giving up on the issue.

At least it's possible to hack/fix the issue when RECT textures are used for
fullscreen PP effects. This includes the filters for MP2, the PP effects used
in HL2 and also the bullettime effects. It won't work that easily for FEAR's
flame effects.

@Stefan: Who would be the correct person to write a request too? I don't think
that simply "complaining" about the issue through the customer service will
have any effect at all.

I was thinking about speaking to Aaron Plattner about this problem, since he
does seem to be in charge for the xf86-video-nv driver. IIRC he's a developer
at NVIDIA, so he should have some contacts with the the developer team of the
proprietary driver.

Greets,
Tobias


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] Max Payne 2: sepia tone filter issues (high PPE)

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #19 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-11-24 14:41:00 ---
*** Bug 15875 has been marked as a duplicate of this bug. ***


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762


Tobias Jakobi <liquid.acid(a)gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Max Payne 2: sepia tone     |GeforceFX series: fullscreen
                   |filter issues (high PPE)    |PP effect issues / RECT
                   |                            |texcoord fixup




--- Comment #20 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-11-24 14:42:58 ---
List of affected games so far: HL2, Max Payne 2, FEAR


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #21 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-11-24 14:44:56 ---
Created an attachment (id=17433)
 --> (http://bugs.winehq.org/attachment.cgi?id=17433)
FEAR 1.08: flame effects not rendered properly


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #22 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-11-24 17:16:49 ---
Created an attachment (id=17438)
 --> (http://bugs.winehq.org/attachment.cgi?id=17438)
Small hack to de-normalize texcoords when used in RECT samplers

This hack just adds a new constant vec2 to the GLSL preamble, containing the
dimensions of the first rendertarget / backbuffer.

This is then used in pshader_glsl_tex to scale the texcoords when a RECT
sampler is used.

This works for me and fixes the issues for all three games. HL2 intro sequence
works, all PP effect for Max Payne 2 work and FEAR no longer has issues for
it's bullettime effect plus the flame effect is also correct now.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #23 from Stefan Dösinger <stefandoesinger(a)gmx.at>  2008-11-24 18:08:40 ---
You can't use the render target size here. You have to use the texture size of
the texture you're sampling from. In all your apps the texture happens to have
the size size as the render target, but that isn't necessarily the case.

You can get the texture sizes from
device->stateblock->texture[i]->currentDesc.(With,Height). You have to keep in
mind though that the texture doesn't have to be a 2D texture(the shader code is
checking that already, otherwise it won't generate a RECT read), or it could be
NULL.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762


Austin English <austinenglish(a)gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch




-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #24 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-11-25 05:26:16 ---
My current assumption is that all RECT textures that are used have the
dimensions of the backbuffer. This assumption holds as long as the game is only
using RECTs for fullscreen post processing effects.

You're right that this won't work (correctly) if the texture where the shader
samples from doesn't have this dimensions.

However currently the most that is sampled from a RECT texture is one single
pixel (since we supply texcoords in the range [0,1]x[0,1]).

This hack isn't valid in general at all, but it's kinda "better" than the
current situation. And after all it's only a hack :)

@"You have to use the texture size of
the texture you're sampling from.":
Seems like my current approach won't work there, since I'm hardcoding the fixup
vector in the shader. I don't think I can simply assume that the shader that is
generated is only used for the currently bound texture (if there is anything
bound at all).

So as soon as the bound texture changes I would have to regenerate the shader,
modifying the fixup vector. That doesn't look like a good/clean approach to me.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #25 from Stefan Dösinger <stefandoesinger(a)gmx.at>  2008-11-25 05:56:28 ---
> Seems like my current approach won't work there, since I'm hardcoding the
> fixup vector in the shader. I don't think I can simply assume that the
> shader that is generated is only used for the currently bound texture (if
> there is anything bound at all).
> So as soon as the bound texture changes I would have to regenerate the
> shader, modifying the fixup vector. That doesn't look like a good/clean
> approach to me.
> You can use a Uniform(shader constants called in d3d. That avoids the
> recompile.

Note that you won't save a uniform by hardcoding the fixup. Shaders don't
support immediate values in the hardware bytecode, so the driver is
transparently consuming a uniform for hardcoded values on its own.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #26 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-11-25 17:54:59 ---
Yeah, thought so that this won't solve the issue with uniform consumption.

I only did choose this approach because I didn't know where wined3d passes
uniforms to the shader.

I see more problems here:
What if a shader program doesn't sample from a single texture but multiple
ones. In that case we even consume more than a single vec2.

So we need at least 2 * nr_of_textures_sampled_from float uniforms here to make
this work.

In any case the uniform state has to be updated when the texture bindings
change. That looks like the hardest part to me, but probably it gets much worse
than I can imagine now :(


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #27 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-11-30 17:52:43 ---
Created an attachment (id=17576)
 --> (http://bugs.winehq.org/attachment.cgi?id=17576)
IRC discussion with Stefan and Henri about the issue

Just some material for me, so I don't forget what I should do to cleanly
implement a fix for this problem.

What I have understood so far:
1) Add new BOOL texrect_fixup to ps_compile_args (in wined3d_private.h) so we
can device whether fixup is needed or not.

Question: Where do I "configure" / initialize the ps_compile_args structure?

2) Add code to sampler() (in state.c) to trigger fixup uniform reloading when
pshaders and rect textures are used.

So I think this is the correct check:
use_ps(stateblock->wineD3DDevice) &&
IWineD3DBaseTexture_GetTextureDimensions(stateblock->textures[sampler]) ==
GL_TEXTURE_RECTANGLE_ARB)

Question: What do I call then? I don't think I can simply call a method from
glsl_shader.c since in the end this whole fix should also kill the issue when
ARB mode is used (and not GLSL).

I suspect I should call shader_backend->load_constants there?
However isn't that call kinda "heavy"? (since I "only" wanna reload the fixup
uniforms).

3) For each sampler2DRect also generate a vec2 uniform containing the texture
dimensions. This is probably going into shader_generate_glsl_declarations
(glsl_shader.c)

Question: Currently I have put this into the texture samplers declaration, but
from the discussion on IRC I assume this has to move into a separate loop.

4) Disable the (now) superfluous texcoord fixup in transform_texture()
(state.c) in case pshaders are used.

What's also a mystery to me is the "register a handler for STATE_SAMPLER(0-16)"
thing, which supposedly located in state.c and arb_program_shader.c. I don't
think that kind of code is found somewhere in sampler() (state.c)

Does this "register"-thing mean adding stuff to the large xxx_template
structures?

Sorry if I'm asking dumb question :(

Greets,
Tobias


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #28 from Stefan Dösinger <stefandoesinger(a)gmx.at>  2008-12-01 10:46:03 ---
> What I have understood so far:
> 1) Add new BOOL texrect_fixup to ps_compile_args (in wined3d_private.h) so we
> can device whether fixup is needed or not.
>
> Question: Where do I "configure" / initialize the ps_compile_args structure?
> In pixelshader.c, find_ps_args(), or similar
> 2) Add code to sampler() (in state.c) to trigger fixup uniform reloading when
> pshaders and rect textures are used.
>
> So I think this is the correct check:
> use_ps(stateblock->wineD3DDevice) &&
> IWineD3DBaseTexture_GetTextureDimensions(stateblock->textures[sampler]) ==
> GL_TEXTURE_RECTANGLE_ARB)
> Correct
> Question: What do I call then? I don't think I can simply call a method from
> glsl_shader.c since in the end this whole fix should also kill the issue when
> ARB mode is used (and not GLSL).
> Correct, you have to use the shader_backend structure
> I suspect I should call shader_backend->load_constants there?
> However isn't that call kinda "heavy"? (since I "only" wanna reload the fixup
> uniforms).
> shader_backend->load_constants is ok for the start. In ARB you can't avoid that
> because you have to prevent the normal constants from overwriting your fixup
> constants(or better: place the fixup constants in a place not used by the
> shader). In GLSL a separate function could do this, but that won't work with
> ARB
> 3) For each sampler2DRect also generate a vec2 uniform containing the texture
> dimensions. This is probably going into shader_generate_glsl_declarations
> (glsl_shader.c)
>
> Question: Currently I have put this into the texture samplers declaration, but
> from the discussion on IRC I assume this has to move into a separate loop.
> No, I think that place is ok. I'm not 100% sure though, put it anywhere where
> you feel its ok
> 4) Disable the (now) superfluous texcoord fixup in transform_texture()
> (state.c) in case pshaders are used.
> Yep
> What's also a mystery to me is the "register a handler for STATE_SAMPLER(0-16)"
> thing, which supposedly located in state.c and arb_program_shader.c. I don't
> think that kind of code is found somewhere in sampler() (state.c)
> I was wrong there, since this is better handled in sampler() in step (2)
> Does this "register"-thing mean adding stuff to the large xxx_template
> structures?
> Yes(although you don't need it, as said above)
> Sorry if I'm asking dumb question :(
> No worries. The questions aren't dumb really :-)

-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #29 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-12-03 10:01:50 ---
Created an attachment (id=17614)
 --> (http://bugs.winehq.org/attachment.cgi?id=17614)
[wined3d] modifications to find_ps_compile_args and ps_compile_args struct

So, something like this should for it for the ps_compile_args side?

(i) Adding the new BOOL to the struct
(ii) Enable as soon as RECTs are used

Or am I missing something here?


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #30 from H. Verbeet <hverbeet(a)gmail.com>  2008-12-03 10:24:19 ---
You'd have to do that for each sampler.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #31 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-12-03 10:42:16 ---
So I can't put it inside the loop?

What are "sampled samplers" anyway? Does this mean they're actually "used", or
am I simply wrong here?

I thought I would suffice to check the texture dimensions for these ones...


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #32 from H. Verbeet <hverbeet(a)gmail.com>  2008-12-03 10:57:52 ---
It has to be inside the loop, but you need to track per-sampler if it needs a
fixup or not. sampled_samplers are the samplers that are actually sampled, yes.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #33 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-12-03 11:03:01 ---
Oh, so this implies that I also need more than a single boolean to store the
fixup need, right?

Something like texrect_fixup[MAX_FRAGMENT_SAMPLERS] ?


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762


Tobias Jakobi <liquid.acid(a)gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #17614|0                           |1
        is obsolete|                            |




--- Comment #34 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-12-03 11:29:31 ---
Created an attachment (id=17617)
 --> (http://bugs.winehq.org/attachment.cgi?id=17617)
[wined3d] modifications to find_ps_compile_args and ps_compile_args struct

Better?


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #35 from H. Verbeet <hverbeet(a)gmail.com>  2008-12-03 11:55:41 ---
Yeah, that should work.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #36 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-12-03 11:57:18 ---
Created an attachment (id=17618)
 --> (http://bugs.winehq.org/attachment.cgi?id=17618)
[wined3d] changes to pshader_glsl_tex (glsl_shader.c)

Next part...


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #37 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-12-03 12:26:20 ---
Created an attachment (id=17619)
 --> (http://bugs.winehq.org/attachment.cgi?id=17619)
[wined3d] changes to sampler() (state.c)

More stuff, again not tested (going to do this later).


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #38 from Stefan Dösinger <stefandoesinger(a)gmx.at>  2008-12-03 12:39:52 ---
In theory your changes look good, I can't promise that it will work though. The
GLSL shader part is incomplete(PsamplerRectFixup is not declared anywhere, and
isn't loaded), and in state.c you're not taking care of the texture matrix side
fixup yet. But all in all it moves into a reasonable direction.

by the way, I think you can ignore vertex samplers. I do not know any card that
supports them, but doesn't support unconditional NP2 textures. (Geforce 6,7
support NP2, ati radeon <= X1999 do not support vertex samplers*, and I think
the GF5 doesn't support vertex samplers either)

(*) Actually, they support them, but you can only use 0 of them concurrently.
That's ATIs dirty trick to get Shader Model 3.0 support certified(where vertex
samplers are mandatory), even though their cards don't support them.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #39 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-12-03 17:38:35 ---
(In reply to comment #38)
> In theory your changes look good, I can't promise that it will work though. The
> GLSL shader part is incomplete(PsamplerRectFixup is not declared anywhere, and
> isn't loaded), and in state.c you're not taking care of the texture matrix side
> fixup yet. But all in all it moves into a reasonable direction.
> Yes, I should have mentioned that the patch(set) isn't complete yet. Still
> working on the rest though. And I also haven't made a compile yet, it's just
> implementing what you told me. If it works.... well, we'll see about that in
> the next week maybe.
> by the way, I think you can ignore vertex samplers. I do not know any card that
> supports them, but doesn't support unconditional NP2 textures. (Geforce 6,7
> support NP2, ati radeon <= X1999 do not support vertex samplers*, and I think
> the GF5 doesn't support vertex samplers either)

OK, but this only concerns the comment above the bitfield, right?


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #40 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-12-04 11:18:15 ---
Created an attachment (id=17634)
 --> (http://bugs.winehq.org/attachment.cgi?id=17634)
[wined3d] even more changes to glsl_shader.c

I think I need a bit of comment on this one.

Doesn't still contain the texture matrix fixup stuff, and isn't even compile
tested. I'd like to know if the changes are basically correct.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #41 from H. Verbeet <hverbeet(a)gmail.com>  2008-12-04 12:41:08 ---
> +            if(prog->rectFixup_location[i]) {
> You need to test against -1 instead of 0 there. For the rest the changes look
> correct. You need to actually use the fixup as well of course, which will
> probably be a bit more interesting.

-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762


Tobias Jakobi <liquid.acid(a)gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #17438|0                           |1
        is obsolete|                            |
  Attachment #17617|0                           |1
        is obsolete|                            |
  Attachment #17618|0                           |1
        is obsolete|                            |
  Attachment #17619|0                           |1
        is obsolete|                            |
  Attachment #17634|0                           |1
        is obsolete|                            |




--- Comment #42 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-12-04 18:06:40 ---
Created an attachment (id=17638)
 --> (http://bugs.winehq.org/attachment.cgi?id=17638)
[wined3d] geforcefx rect patchset (incomplete)

Still incomplete patchset for the issue.

I think I need more help here. The problem is that it "kinda" works.

What means that as long as I "hardcode" tex_dim to {800.0, 600.0} in
shader_glsl_load_constants it works. Effects rendered fine, no GL errors,
compilation warningless, etc.

However...
const float tex_dim[2] = {surf->currentDesc.Width, surf->currentDesc.Height};

This does not seem to produce the results I would like to have.

That's what is stored there:
dims: 0.000000 , 1.000000

This surely can't be the backbuffer dimensions...
I think I just can't do the case there, and end up getting wrong data (from a
wrong memory adress) - but not so sure about that.

Any ideas?


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #43 from Stefan Dösinger <stefandoesinger(a)gmx.at>  2008-12-04 18:14:43 ---
I'll look at your code, but I don't think you want to access an
IWineD3DSurfaceImpl anywhere here. You probably have an IWineD3DBaseTexture *
and are casting it to an IWineD3DSurfaceImpl *, which as you guessed is not
correct. If you know that it is a RECT texture, you know you have an
IWineD3DTexture *, so you can safely cast it to IWineD3DTextureImpl *. In there
you have the np2 correction matrix, and you'll want to use element 0 and 5 from
it.

This is the matrix:

w 0 0 0
0 h 0 0
0 0 1 0
0 0 0 1

This shows why you want elements 0 and 5


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #44 from Stefan Dösinger <stefandoesinger(a)gmx.at>  2008-12-04 18:42:41 ---
Yes, this is the problem:

/* can we safely assume that the cast works here? */
const IWineD3DSurfaceImpl *surf =
(IWineD3DSurfaceImpl*)stateBlock->textures[i];
const float tex_dim[2] = {surf->currentDesc.Width, surf->currentDesc.Height};

The cast is not safe, because a texture is not a surface. It is a container
that contains one or more surfaces(or volumes, in case of a 3D texture).

I also noticed this:
if(arg->opcode_token & WINED3DSI_TEXLD_BIAS) {
    ...
+    /* what to do here when texrect is enabled? */
    shader_addline(arg->buffer, "%s(Psampler%u, %s, %s)%s);\n",
}

I'd argue that this should never happen, because D3DSI_TEXLD_BIAS specifies a
level of detail bias to use a different mipmap sublevel(see below), which
doesn't make much sense with RECT textures since they only have one level
anyway. However, it could happen that D3D supports this, and simply ignores the
LOD Bias in this case. I think this is a candidate for a test case.

FYI:
A 1024x1024 2D texture can for example contain 10 surfaces, which are then
usually called mipmap sublevels. The first one has 1024x1024 pixels, the 2nd
512x512, then 256x256 etc. Depending on the level of detail needed when
sampling from the texture, different a different mipmap is read. This reduces
filtering artifacts because the artist can draw the smaller mipmaps separately
and optimize the content for looking well instead of relying on strictly
mathematical filtering algorithms. (Ever played an old game where fences,
bricks or other recursive patterns start flickering or show other odd effects?)

Don't worry: You don't have to care about mipmaps here. Why?

There are also cube textures. Those essentially 6 2D textures(possibly with
mipmaps) arranged in a cubic layout. When sampling from such a texture, you sit
in the center of the cube, and a 3D direction vector points to a texel. This is
often used for drawing reflections.

If you do need a surface in this code


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #45 from Stefan Dösinger <stefandoesinger(a)gmx.at>  2008-12-04 19:20:58 ---
I forgot to add a few things:

The patch looks reasonable, appart of the things noted above here are a few
tiny recommendations:

+        for(i = 0; i < MAX_FRAGMENT_SAMPLERS; ++i) {
+            if(compile_args.texrect_fixup & (1 << i)) {
+                sprintf(name, "PsamplerRectFixup%d", i);
+                entry->rectFixup_location[i] =
GL_EXTCALL(glGetUniformLocationARB(programId, name));
+            } else
+                entry->rectFixup_location[i] = -1;

This is not needed, glGetUniformLocationARB returns -1 if the name isn't found.
So GL does this check for you. I personally have some problems if havin {} in
one of case and not in the other, but that's mostly my personal opinion. When I
looked at the code I thought that a } is missing there

+        if(GL_TEXTURE_RECTANGLE_ARB ==
IWineD3DBaseTexture_GetTextureDimensions(stateblock->textures[sampler]))
+             args->texrect_fixup |= (1 << sampler);
The line above that adds trailing whitespace

+        /* trigger shader constant reloading (for texture coordinate fixup,
when RECT samplers are used) */
+        if(GL_TEXTURE_RECTANGLE_ARB ==
IWineD3DBaseTexture_GetTextureDimensions(stateblock->textures[sampler]) &&
I think, but I haven't checked, that the code around this place also uses
GetTextureDimensions in a few cases. It may be an idea to read it once and
store it in a local variable to avoid calling the COM method over and over.

Also, my unfinished sentence: If you really need a surface in the glsl_shader.c
code, you have to use IWineD3DTexture_GetSurfaceLevel or
IWineD33DBaseTextureImpl::surfaces[level]. I don't think you need it though

Your patch also ignores one case: When a vertex shader is used(texcoord mul
doesn't apply), but no pixel shader is used(your new code doesn't apply). In
this case the texrect will still be broken, but I think it is better to ignore
this for now. It is broken now, it will be broken after your patch, so things
don't get worse, but your patch fixes the much more common case of vertex+pixel
shader used.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #46 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-12-05 02:54:49 ---
Hi Stefan!

(In reply to comment #43)
> I'll look at your code, but I don't think you want to access an
> IWineD3DSurfaceImpl anywhere here. You probably have an IWineD3DBaseTexture *
> and are casting it to an IWineD3DSurfaceImpl *, which as you guessed is not
> correct. If you know that it is a RECT texture, you know you have an
> IWineD3DTexture *, so you can safely cast it to IWineD3DTextureImpl *. In there
> you have the np2 correction matrix, and you'll want to use element 0 and 5 from
> it.
> I'm using a cast to IWineD3DTextureImpl now, which does seem to work:
> const IWineD3DTextureImpl *tex = (IWineD3DTextureImpl*)stateBlock->textures[i];

However IWineD3DTextureImpl doesn't contain a np2 correction matrix. I'm using
the width and height members instead, which seem to contain the correct
information. At least the dimensions printed by the debug code are now correct
(backbuffer dims).
> This is the matrix:
>
> w 0 0 0
> 0 h 0 0
> 0 0 1 0
> 0 0 0 1
>
> This shows why you want elements 0 and 5
> I assume this is the matrix normally used when doing the fixup in the fixed
> function pipeline?

(In reply to comment #44)
> Yes, this is the problem:
>
> /* can we safely assume that the cast works here? */
> const IWineD3DSurfaceImpl *surf =
> (IWineD3DSurfaceImpl*)stateBlock->textures[i];
> const float tex_dim[2] = {surf->currentDesc.Width, surf->currentDesc.Height};
>
> The cast is not safe, because a texture is not a surface. It is a container
> that contains one or more surfaces(or volumes, in case of a 3D texture).
> Yeah, I'm still a bit confused by all the terminology used :)
> Thanks for explaining!
> I also noticed this:
> if(arg->opcode_token & WINED3DSI_TEXLD_BIAS) {
>     ...
> +    /* what to do here when texrect is enabled? */
>     shader_addline(arg->buffer, "%s(Psampler%u, %s, %s)%s);\n",
> }
>
> I'd argue that this should never happen, because D3DSI_TEXLD_BIAS specifies a
> level of detail bias to use a different mipmap sublevel(see below), which
> doesn't make much sense with RECT textures since they only have one level
> anyway. However, it could happen that D3D supports this, and simply ignores the
> LOD Bias in this case. I think this is a candidate for a test case.
> OK, I keep the testcase thing in mind, but I prefer to first get the rest
> running. I think I just drop a FIXME there if texrect is enabled. This way we
> at least see when this situation does happen or if it happens at all.
> FYI:
> A 1024x1024 2D texture can for example contain 10 surfaces, which are then
> usually called mipmap sublevels. The first one has 1024x1024 pixels, the 2nd
> 512x512, then 256x256 etc. Depending on the level of detail needed when
> sampling from the texture, different a different mipmap is read. This reduces
> filtering artifacts because the artist can draw the smaller mipmaps separately
> and optimize the content for looking well instead of relying on strictly
> mathematical filtering algorithms. (Ever played an old game where fences,
> bricks or other recursive patterns start flickering or show other odd effects?)
> Yeah, Nyquist-Shannon sampling theorem and all that stuff. I have some
> literature around (Computer Graphics - Principles and Practices), so I think I
> know at least the basics. :)
> Oh yes, you forgot a important advantage of mipmaps: texture cache trashing ;)
> Don't worry: You don't have to care about mipmaps here. Why?
>
> There are also cube textures. Those essentially 6 2D textures(possibly with
> mipmaps) arranged in a cubic layout. When sampling from such a texture, you sit
> in the center of the cube, and a 3D direction vector points to a texel. This is
> often used for drawing reflections.
>
> If you do need a surface in this code
> I'm not sure if I understand what you're trying to tell me here? IIRC
> arb_tex_rect does only "work" with 2D targets anyway. It's not specified for
> 1D, 3D and cube targets, so...?

(In reply to comment #45)
> I forgot to add a few things:
>
> The patch looks reasonable, appart of the things noted above here are a few
> tiny recommendations:
>
> +        for(i = 0; i < MAX_FRAGMENT_SAMPLERS; ++i) {
> +            if(compile_args.texrect_fixup & (1 << i)) {
> +                sprintf(name, "PsamplerRectFixup%d", i);
> +                entry->rectFixup_location[i] =
> GL_EXTCALL(glGetUniformLocationARB(programId, name));
> +            } else
> +                entry->rectFixup_location[i] = -1;
>
> This is not needed, glGetUniformLocationARB returns -1 if the name isn't found.
> So GL does this check for you. I personally have some problems if havin {} in
> one of case and not in the other, but that's mostly my personal opinion.
> No, in fact this is very much needed. I also first had it without the
> else-part, but this crashes most apps early. The thing is that
> glGetUniformLocationARB is ONLY called when the bit in texrect_fixup set. So if
> this bit changes from set to not-set and the above-forloop is called again it
> does not overwrite the data that was previously there. The old data is still
> there, which causes chaos (stateBlock->textures[i] suddenly becomes NULL or has
> non-RECT type).
> So if the bit is not set I have to clear rectFixup_location[i], otherwise we'll
> end up with invalid data here.
> When I
> looked at the code I thought that a } is missing there
>
> +        if(GL_TEXTURE_RECTANGLE_ARB ==
> IWineD3DBaseTexture_GetTextureDimensions(stateblock->textures[sampler]))
> +             args->texrect_fixup |= (1 << sampler);
> The line above that adds trailing whitespace
> Fixed :)
> +        /* trigger shader constant reloading (for texture coordinate fixup,
> when RECT samplers are used) */
> +        if(GL_TEXTURE_RECTANGLE_ARB ==
> IWineD3DBaseTexture_GetTextureDimensions(stateblock->textures[sampler]) &&
> I think, but I haven't checked, that the code around this place also uses
> GetTextureDimensions in a few cases. It may be an idea to read it once and
> store it in a local variable to avoid calling the COM method over and over.

I looked through the surrounding code in the function, but there are no more
calls to that COM method.
> Also, my unfinished sentence: If you really need a surface in the glsl_shader.c
> code, you have to use IWineD3DTexture_GetSurfaceLevel or
> IWineD33DBaseTextureImpl::surfaces[level]. I don't think you need it though
> Yeah, looks like it works properly just with casting to IWineD3DTextureImpl*,
> which I'm doing at the moment.
> Still I have only tested the code with MP2, so there might be some more hidden
> issues (going to test this later with FEAR and HL2).
> Your patch also ignores one case: When a vertex shader is used(texcoord mul
> doesn't apply), but no pixel shader is used(your new code doesn't apply). In
> this case the texrect will still be broken, but I think it is better to ignore
> this for now. It is broken now, it will be broken after your patch, so things
> don't get worse, but your patch fixes the much more common case of vertex+pixel
> shader used.
> Yes, so I'm going to leave this open until I have the remaining issues fixed.

Greets,
Tobias


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762


Tobias Jakobi <liquid.acid(a)gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #17638|0                           |1
        is obsolete|                            |




--- Comment #47 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-12-05 03:19:00 ---
Created an attachment (id=17650)
 --> (http://bugs.winehq.org/attachment.cgi?id=17650)
[wined3d] geforcefx rect patchset (incomplete)

Next revision


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #48 from Tobias Jakobi <liquid.acid(a)gmx.net>  2008-12-05 04:49:52 ---
Just tested this with HL2 and it really breaks the game.

I get both GL errors and visual glitches. Still trying to figure out what does
wrong there.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #49 from Stefan Dösinger <stefandoesinger(a)gmx.at>  2008-12-05 05:05:24 ---
(In reply to comment #46)
> However IWineD3DTextureImpl doesn't contain a np2 correction matrix. I'm using
> the width and height members instead, which seem to contain the correct
> information. At least the dimensions printed by the debug code are now correct
> It is in the BaseTextureClass structure:
> IWineD3DTextureImpl->baseTexture.np2Matrix. A IWineD3DBaseTextureImpl * is ok
> here too in fact. Its our hacky way of setting up inheritance without a C++
> compiler

The NP2 fixup matrix is in all 3 texture types because we also have NP2
emulation with cube and volume textures. E.g. a 200x200x200 cube texture is
scaled up to 256x256x256. However, I think we can drop this code because
Windows D3D does conditional NP2 textures only with 2D textures. So you don't
have to care about cube and volume textures here.
> > This is the matrix:
> >
> > w 0 0 0
> > 0 h 0 0
> > 0 0 1 0
> > 0 0 0 1
> >
> > This shows why you want elements 0 and 5
> > I assume this is the matrix normally used when doing the fixup in the fixed
> > function pipeline?
> > Correct. It is set when creating the texture.
> I'm not sure if I understand what you're trying to tell me here? IIRC
> arb_tex_rect does only "work" with 2D targets anyway. It's not specified for
> 1D, 3D and cube targets, so...?
> I just wanted to elaborate why a texture structure cannot be a surface, even
> though they seem very related in the 2D case. But since you know the 3D
> graphics stuff I can stop this lecturing :-)
> No, in fact this is very much needed. I also first had it without the
> else-part, but this crashes most apps early.
> I thought about dropping the if entirely and call GetUniform unconditionally,
> but maybe I am missing something here. Having the if doesn't really hurt
> though, since that code shouldn't be performance critical.

-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762


Tobias Jakobi <liquid.acid(a)gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #17650|0                           |1
        is obsolete|                            |




--- Comment #50 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-01-24 18:28:14 ---
Created an attachment (id=18971)
 --> (http://bugs.winehq.org/attachment.cgi?id=18971)
geforcefx rect patchset (incomplete)

Hi there,

sorry - I didn't have lot of time lately to work on any patches. Now that my
oral exam is over I did return to this patch and rebased it against current git
master.

Seems like the code works flawlessly now. I have correct rendering behaviour
now in Max Payne 2 as well as in HL2. Also no new error messages or rendering
artifacts popping up in HL2 as it was the case when I tried the patch
previously.

Still have to try FEAR though. However it's great to see that during my absence
wine's code has changed so much that my code works perfectly now :D

Current issues with the patch:
- only works in GLSL mode, lacks code for ARB mode
- only works when pixelshaders are used, it doesn't provide code for a
vertexshader-only usage environment

@Stefan: Could you take a(nother) look at the patch and give me your
impressions about the quality? Is it ready to be pushed to wine-patches even
with the two remaining issues?
And do you see any additional problems?

I plan to solve the issue also for ARB mode, but probably it's a lot harder
there since I can't use high-level shading syntax.

Greets,
Tobias


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #51 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-01-25 10:39:10 ---
Created an attachment (id=18987)
 --> (http://bugs.winehq.org/attachment.cgi?id=18987)
hack to get this working in arb mode

Tried to fiddle around a bit with the arb code. Hacked this together. It's only
working with fixed fb dimensions since I haven't figured out yet how to pass
params inside arbfp programs.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #52 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-01-31 18:20:13 ---
Hi there,

haven't yet managed to produce any working code for ARB mode, but I think I'm
getting closer.

Like Stefan told me I should do the constant loading for texrect fixup similar
to that of the bump env matrix.

What I have figured out so far:

(1) I have to change shader_arb_load_constants, checking there if there are any
rect textures used in the shader and if yes, also check if fixup is possible
(maybe we were already out of constants).
If both checks pass I'll proceed with glProgramEnvParameter2fv to push the
texture dimensions into the program.

PROBLEM: What's the easiest way to get access to the ps_compile_args structure
inside shader_arb_load_constants. Wasn't a big thing with GLSL, but I'm not
really sure which structure that is available from inside the function leads me
to ps_compile_args.

(2) Change shader_generate_arb_declaration:
Similar to bump env matrix loading I carefully check if we're not already out
of free constants. If there are still constants free I proceed to add "PARAM
textureRectFixup..." lines into the shader code.

Stefan already told me that all data inside an ARB program is of type vec4, so
I should probably come up with a way to not burn a full vec4 PARAM for only one
texture. So think about a good way of storing two fixup vec2s inside one PARAM.

(3) Change shader_hw_sample: Check whether we're sampling from RECT and if
constants for this RECT could be loaded. If yes, then scale coords and so on.

(4) Not sure about this: gen_arbfp_ffp_shader.
Since it contains texture sampling instructions that can encounter RECT types,
it should probably be changed as well, right?

Upon finishing the fixup thing with ARB I'm going to move on to also fixing
this for vertexshader-only setups (Stefan told me before that this could also
be the case, and currently the whole fixup thing only works when pixelshaders
are used).

Greets,
Tobias


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #53 from Stefan Dösinger <stefandoesinger(a)gmx.at>  2009-02-01 17:24:13 ---
> PROBLEM: What's the easiest way to get access to the ps_compile_args structure
> inside shader_arb_load_constants. Wasn't a big thing with GLSL, but I'm not
> really sure which structure that is available from inside the function leads me
> to ps_compile_args.
> I've no quick idea. The idea behind the compile_args struct is to be used only
> during compilation, so the answer is probably you don't access it(neither in
> GLSL, although there it is easy to use it). Probably fetch some values from the
> stateblock, and/or shadow them in the compiled_shader structure.

(actually, you may access the compile_args struct in a compiled_shader member,
but don't try to access the same pointer that is passed to compile_shader)
> (4) Not sure about this: gen_arbfp_ffp_shader.
> Since it contains texture sampling instructions that can encounter RECT types,
> it should probably be changed as well, right?
> You don't have to do anything there necessarily. The vertex pipeline is
> supposed to take care of this if no d3d pixel shader is used.

However, you could add some fixup code there too instead of fixing the coords
in the vertex shader. Note that for fully correct operation you still need to
be able to handle the fixups in the vertex shader, but keeping it in the pixel
shader and fp ffp replacement may be easier and keep the state management
simpler. If it keeps the code simpler I'd be willing to say that an ffp
replacement is needed for proper RECT operation with vertex shaders(all cards
that have vshaders have either atifs, arbfp or nvts) The question is how this
affects performance. We're adding extra instructions and probably a dependent
read to the fragment processing, but avoid interdependencies between vertex and
fragment pipeline.

I also don't know if we can handle the RECT fixup in atifs or nvts. They only
have 8 instructions and 8 constants, and may not even be able to perform
dependent reads on RECT textures.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762


Tobias Jakobi <liquid.acid(a)gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #18971|0                           |1
        is obsolete|                            |




-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762


Tobias Jakobi <liquid.acid(a)gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #18987|0                           |1
        is obsolete|                            |




-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #54 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-02-01 18:09:33 ---
Created an attachment (id=19177)
 --> (http://bugs.winehq.org/attachment.cgi?id=19177)
[01] [wined3d] add glGetProgram{Env,Local}ParameterfvARB to wined3d_gl header


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #55 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-02-01 18:09:54 ---
Created an attachment (id=19178)
 --> (http://bugs.winehq.org/attachment.cgi?id=19178)
[02] [wined3d] add texrect_fixup bitfield to ps_compile_args structure


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #56 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-02-01 18:10:10 ---
Created an attachment (id=19179)
 --> (http://bugs.winehq.org/attachment.cgi?id=19179)
[03] [wined3d] initialize new texrect_fixup bitfield properly in pixelshader.c


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #57 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-02-01 18:10:27 ---
Created an attachment (id=19180)
 --> (http://bugs.winehq.org/attachment.cgi?id=19180)
[04] [wined3d] implement texrect coord fixup in GLSL mode


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #58 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-02-01 18:10:44 ---
Created an attachment (id=19181)
 --> (http://bugs.winehq.org/attachment.cgi?id=19181)
[05] [wined3d] trigger constant reloading when rect textures change


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #59 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-02-01 18:10:58 ---
Created an attachment (id=19182)
 --> (http://bugs.winehq.org/attachment.cgi?id=19182)
[06] [wined3d] add texrect fixup structures to shader_arb_priv


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #60 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-02-01 18:11:13 ---
Created an attachment (id=19183)
 --> (http://bugs.winehq.org/attachment.cgi?id=19183)
[07] [wined3d] implement texrect coord fixup in ARB mode


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #61 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-02-01 18:11:31 ---
Created an attachment (id=19184)
 --> (http://bugs.winehq.org/attachment.cgi?id=19184)
[08] [wined3d] various constifications in arb_program_shader.c


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762


Tobias Jakobi <liquid.acid(a)gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #19177|0                           |1
        is obsolete|                            |
  Attachment #19178|0                           |1
        is obsolete|                            |
  Attachment #19179|0                           |1
        is obsolete|                            |
  Attachment #19180|0                           |1
        is obsolete|                            |
  Attachment #19181|0                           |1
        is obsolete|                            |
  Attachment #19182|0                           |1
        is obsolete|                            |
  Attachment #19183|0                           |1
        is obsolete|                            |
  Attachment #19184|0                           |1
        is obsolete|                            |




--- Comment #62 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-03-05 16:18:35 ---
Created an attachment (id=19811)
 --> (http://bugs.winehq.org/attachment.cgi?id=19811)
the patches (this time in an archive)

should now again build against current git master


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #63 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-03-06 18:00:06 ---
@stefand: Could you maybe take a look at the current patchset? I think at least
the GLSL part could go into git


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #64 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-03-28 15:11:34 ---
Created an attachment (id=20170)
 --> (http://bugs.winehq.org/attachment.cgi?id=20170)
New IRC discussion log (this time about ARB impl)

The np2 fixup works really good with GLSL currently, but ARB is still a major
problem (which I want to fix before moving on to a faster card *g*).

This here is the IRC discussion with stefand about the problem. It's mainly a
hint for me how to do it correctly.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #65 from H. Verbeet <hverbeet(a)gmail.com>  2009-03-29 07:01:33 ---
Actually, wrt. constant reloading, the way to do that in the long term would be
to introduce the concept of constant buffers (d3d10) to the backends. Our
internal fixups would be in their own buffer separate from application
constants in that case. That's not completely trivial to implement though.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #66 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-04-08 21:09:09 ---
I looked into what Stefan told me, and I don't think it's going to work.

Quote from the ARB_fragment_program docs:

The error INVALID_VALUE is generated by commands
ProgramEnvParameter{fd}ARB, ProgramEnvParameter{fd}vARB, and
GetProgramEnvParameter{fd}vARB if <index> is greater than or equal
to the value of MAX_PROGRAM_ENV_PARAMETERS_ARB corresponding to the
program target <target>.

Like Stefan mentioned MAX_PROGRAM_ENV_PARAMETERS_ARB is usually 256.

Even if it would work to put something like "PARAM ThisIsMyPrivateThing =
{programs.env[256 + 1]};" into a fragment/vertex program, there is no way to
reliably fill the PARAM with data.

The docs clearly state that this would trigger an error. I'm very much against
exploiting some driver functionality that may be there (I haven't actually
tested what Stefan told me), but is potentially unstable and only exists for
some drivers / some GPUs.

Or am I misunderstanding something here??


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762


Tobias Jakobi <liquid.acid(a)gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #19811|0                           |1
        is obsolete|                            |




-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #67 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-04-09 15:47:00 ---
Created an attachment (id=20353)
 --> (http://bugs.winehq.org/attachment.cgi?id=20353)
[wined3d] ARB np2fixup patchset (beta 2)

This is beta2 of the np2fixup patchset for ARB mode.

It still doesn't work with HL2 though. The implementation should be pretty much
correct, but np2 constant reloading in ARB doesn't seem to work the way I want.

For example the correct values are loaded into the env PARAMs, but they don't
seems to reach the shader. Something probably does overwrite them before the
shader code is executed. Don't know why though. Or what could overwrite them.

This could possibly be solved by switching to local PARAMs, but this has to
wait. stefand doesn't want to have this code merged with the current state of
the ARB codebase.

Again, the code should pretty much work - but if you want it to work properly
you have to figure out why the env params get overwritten before the shader can
use it.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #68 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-04-09 18:59:46 ---
Another note for me:
Port over the param/uniform "squeezing" code from ARB to GLSL. According to
stefand the GLSL compilers don't seem very bright when it comes to efficiently
packing vec2 uniforms. Looks like a vec2 uniform is just mapped to a vec4,
essentially wasting a lot of space.

Since the squeezing code should both be implemented in GLSL and ARB I might
introduce some patches from the ARB patchset, since they add the
data-structures needed for the squeeze.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762


Tobias Jakobi <liquid.acid(a)gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #20353|0                           |1
        is obsolete|                            |




-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #69 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-04-18 14:09:43 ---
http://download.nvidia.com/developer/Papers/2005/OpenGL_2.0/NVIDIA_OpenGL_2....

According to this the NV30 (GeforceFX) series doesn't support vertex texture
fetching in hardware, so a fixup for NP2 textures won't be necessary here.

I have the constant packing code for GLSL ready (going to submit this once
Henri's patches are in, since I don't want too much collision) and also a
working ARB patchset.
The only thing missing is NP2 fixup with a vertexshader-only configuration. I'm
only going to implement this for GLSL, since it's probably going to get ugly
with ARB. Once this is done, the bug will be closed (doesn't make much sense to
implement more corner cases which won't ever appear on the FX).


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762





--- Comment #70 from Tobias Jakobi <liquid.acid(a)gmx.net>  2009-07-04 06:13:19 ---
Short status update:
(i) The constant packing code for GLSL is in git master, this should help with
applications that use a lot of NP2 textures - however most apps don't do that
and the FX won't run them at acceptable framerate anyway. The code still needs
fallback implementation, which means that it should disable the fixup if the
shader would fail because of too many uniforms (shader constants in GLSL) used.
This isn't done currently since the used uniforms are counted nowhere.

(ii) NP2 fixup code for ARB also went into git master - this already implements
constant packing and a fallback. Fallback was done because unsuccessfully
compiled shaders generate a lot of GL error noise when wine attempts to draw
with them. So we just disable the fixup once we run out of constants, peroid.
This isn't going to look that good but at least it "works" to some extent.
However I have yet to see a game that triggers the fallback.

(iii) All NP2 fixup code is currently exclusive to pixelshader mode, it won't
kick in when there is only a vertexshader active. A GLSL implementation for
vshader-only mode is planned - ARB.... maybe, but not very likely.

(iv) A testcase is still missing. Going to write one soon....

-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762

Henri Verbeet <hverbeet(a)gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
    Regression SHA1|                            |d09cbcec91561186bee77aaba55
                   |                            |b29cbf2d7e7ef

-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762

Stefan Dösinger <stefandoesinger(a)gmx.at> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #71 from Stefan Dösinger <stefandoesinger(a)gmx.at> 2011-08-18 18:03:24 CDT ---
I'll resolve this bug, RECT fixup with shaders is basically there. Any further
problems should be reported as new bugs. I think the main remaining problem is
making sure that this functionality doesn't break now that most GPUs out there
have proper unconditional non power of two support.

-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 14762] GeforceFX series: fullscreen PP effect issues / RECT texcoord fixup

http://bugs.winehq.org/show_bug.cgi?id=14762

Alexandre Julliard <julliard(a)winehq.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #72 from Alexandre Julliard <julliard(a)winehq.org> 2011-08-26 13:23:43 CDT ---
Closing bugs fixed in 1.3.27.

-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.