Bug 61091

glBlitFramebuffer with rectangle textures is also broken with both softpipe and llvmpipe and it has been so for quite a while. I have a piglit test for that.

(In reply to comment #1)
> glBlitFramebuffer with rectangle textures is also broken with both softpipe
> and llvmpipe and it has been so for quite a while. I have a piglit test for
> that.

The problem is that u_blitter is misleading drivers: it always sets sampler_state.normalized_coords = 1, but then passes unnormalized coords for rectangle textures.

It effectively only works on drivers that ignore normalized_coords.

If you prefer passing unnormalized coord then please fix the state. Alternatively we could easily modify u_blitter to always pass normalized coords.

Another thing to consider is just remove normalized_coords state, and force drivers follow the texture target.

(In reply to comment #2)
> (In reply to comment #1)
> > glBlitFramebuffer with rectangle textures is also broken with both softpipe
> > and llvmpipe and it has been so for quite a while. I have a piglit test for
> > that.
> 
> The problem is that u_blitter is misleading drivers: it always sets
> sampler_state.normalized_coords = 1, but then passes unnormalized coords for
> rectangle textures.
> 
> It effectively only works on drivers that ignore normalized_coords.
> 
> If you prefer passing unnormalized coord then please fix the state.
> Alternatively we could easily modify u_blitter to always pass normalized
> coords.
> 
> Another thing to consider is just remove normalized_coords state, and force
> drivers follow the texture target.

The normalized_coords bit is required for OpenCL (see clCreateSampler), but we should first define the interaction between pipe_texture_target and normalized_coords. I propose this behavior:


target   | normalized_coords | behavior
---------------------------------------------
RECT     | false             | unnormalized
RECT     | true              | unnormalized
non-RECT | false             | unnormalized
non-RECT | true              | normalized


That is, the normalized_coords bit should only affect non-rect textures. This behavior maps nicely to HD 6900 and later radeon hardware. The hardware respects the texture target specified by the TEX instruction and can optionally force unnormalized coords in the sampler state if needed. Older radeon hardware doesn't have the sampler state bit and must recompile shaders, so setting normalized_coords != 1 should be avoided if possible.

(In reply to comment #3)
> The normalized_coords bit is required for OpenCL (see clCreateSampler), 

Pitty.

> but
> we should first define the interaction between pipe_texture_target and
> normalized_coords. I propose this behavior:
> 
> 
> target   | normalized_coords | behavior
> ---------------------------------------------
> RECT     | false             | unnormalized
> RECT     | true              | unnormalized
> non-RECT | false             | unnormalized
> non-RECT | true              | normalized
> 
> 
> That is, the normalized_coords bit should only affect non-rect textures.
> This behavior maps nicely to HD 6900 and later radeon hardware. The hardware
> respects the texture target specified by the TEX instruction and can
> optionally force unnormalized coords in the sampler state if needed. Older
> radeon hardware doesn't have the sampler state bit and must recompile
> shaders, so setting normalized_coords != 1 should be avoided if possible.

I don't a strong opinion either way. I think that for softpipe/llvmpipe it's relatively trivial either. Please post your proposal on mesa-dev so that other hardware driver maintainers can comment whether this will cause problems or not.

(In reply to comment #3)
> 
> target   | normalized_coords | behavior
> ---------------------------------------------
> RECT     | false             | unnormalized
> RECT     | true              | unnormalized
> non-RECT | false             | unnormalized
> non-RECT | true              | normalized
> 
> 
> That is, the normalized_coords bit should only affect non-rect textures.
> This behavior maps nicely to HD 6900 and later radeon hardware. The hardware
> respects the texture target specified by the TEX instruction and can
> optionally force unnormalized coords in the sampler state if needed. Older
> radeon hardware doesn't have the sampler state bit and must recompile
> shaders, so setting normalized_coords != 1 should be avoided if possible.

I guess we could change it to that, though note this was definitely not the intention initially. The docs mention rect targets "must use unnormalized coords" whereas 2d targets "must use normalized coords" - it actually looks like opencl was supposed to use different targets depending if the coords were normalized or not. (But it won't work since we don't have different targets for 3d textures.)
That is definitely not the same as ignoring normalized bits.

For llvmpipe it actually would be not very natural changing this so I'm not a big fan of having inconsistent normalized state. (Though yes instead of querying normalized bit coming from sampler state we could quite easily derive some new normalized bit depending on target.). For softpipe it's a similar story.
OTOH since RECT was mostly introduced for hw which couldn't do normalized coords with NPOT texures anyway I could see there's some point.

I really want this fixed soon, so lets first fix this using the current semantics -- it's just a matter of creating/using a few more sampler state objects --, and then take our time deciding if/how to improve them.

I'm on it.

Nevermind. I just noticed the fix (fdb88967e3a1d559fa522afa8336446e92330ff) had been committed already.

(In reply to comment #8)
> Nevermind. I just noticed the fix (fdb88967e3a1d559fa522afa8336446e92330ff)
> had been committed already.

Oops. I thought I had commented and resolved when I submitted the fix. Sorry.