Textures not tiling properly

[Jaenis]

I have tiling texture (get it here if interested) but it will not tile properly.
Image size is 256*256.

Here's relevant parts of code

Code: [Select]

// Load image
sf::Image Image;
if (!Image.LoadFromFile("Test.png"))
return EXIT_FAILURE;
int Width=Image.GetWidth();
int Height=Image.GetHeight();
sf::Sprite Sprite(Image);

...

// Draw the sprite
for (int y=0; y<3; y++)
for (int x=0; x<3; x++)
{
Sprite.SetPosition(x*Width,y*Height);
App.Draw(Sprite);
}

The result is this:


As you can see, there are seams on the texture

The reason for this can be found from SFML, at src\SFML\Graphics\Image.cpp, line 658 (in SFML2 branch)

Code: [Select]


        GLCheck(glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP));
        GLCheck(glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP));


When I replace that GL_CLAMP with GL_CLAMP_TO_EDGE, I'll get this image instead:


This could be graphics card dependent, I have ATI Radeon 4890, but I think that is not the reason.
Here's more about the subject

Anyhow, that GL_CLAMP should be replaced with GL_CLAMP_TO_EDGE

[Laurent]

Which version of SFML are you using?

GL_CLAMP_TO_EDGE is not a good solution, for two reasons:
- It requires OpenGL 1.2
- You still get one extra pixel, it's just less noticeable because it has the same color as its neighbour

Another way to solve this problem is to disable bilinear filtering (Image.SetSmooth(false)).

There may be another fix, internal this time, but I first need to know your version of SFML

[Jaenis]

I found this with SFML2 branch from SVN

[Laurent]

Ok :cry:

A few weeks ago I implemented a trick to get pixel perfect rendering, which was supposed to get rid of all rendering artifacts. I did almost every test I could think of, and they all rendered perfectly with the new fix.
Now it seems that you've found one situation where the rendering is broken ; if I remove the trick it renders perfectly. If I restore the other trick I was using before (offsetting the texture coordinates by half a pixel), it works as well.
It seems there's no global solution, everytime I (think I) fix the code, new problems arise :lol:

[l0calh05t]

[...]
If I restore the other trick I was using before (offsetting the texture coordinates by half a pixel), it works as well.

The texel center of a texel i in a texture of width N lies at (0.5 + i)/N. So if we have a 16x16 sprite quad (positioned correctly) with 0..1 texture coordinates and a 16x16 texture, the texel centers should coincide with the screen pixel centers. At least that's the theory...

Obviously with a 32x32 quad and a 16x16 texture, the border pixels would be interpolated... and for a non-repeating texture (like a sprite) clamp would be the correct behavior, but for a repeating texture it would be repeat. As far as I can tell you use the same border behavior for all textures, so it can't work in all situations.

It seems there's no global solution, everytime I (think I) fix the code, new problems arise :lol:

Seems familiar...

[Jaenis]

GL_CLAMP_TO_EDGE is not a good solution, for two reasons:
- It requires OpenGL 1.2
- You still get one extra pixel, it's just less noticeable because it has the same color as its neighbour

Actually GL_REPEAT fixes this better than that CLAMP_TO_EDGE.
Clamp had extra pixel as you said, but repeat displayed everything perfectly. Tried this with "chessboard" texture.

How about if you give an interface to change between GL_REPEAT / GL_CLAMP, like you already have Image::SetSmooth(bool).
Since sprites are commonly with clamp and level textures with repeat, having one forced to you isn't so nice.
For example Image::SetRepeat(bool) would solve this.


Also a bit offtopic, why OpenGL 1.2 would be a restriction? :wink:

[dabo]

Also a bit offtopic, why OpenGL 1.2 would be a restriction? :wink:

To support crappy old integrated graphics chipsets perhaps. Like my Intel GMA 950, but I can actually use version 1.4

[Laurent]

How about if you give an interface to change between GL_REPEAT / GL_CLAMP, like you already have Image::SetSmooth(bool).
Since sprites are commonly with clamp and level textures with repeat, having one forced to you isn't so nice.
For example Image::SetRepeat(bool) would solve this.

Not an option. SFML may create internal textures padded with white pixels when the GPU doesn't support non-power-of-two dimensions.

Also a bit offtopic, why OpenGL 1.2 would be a restriction?

Because some old cards doesn't support it, unfortunately. Even if there are only few of them, I feel bad rejecting them just because of a flag definition

[Jaenis]

Not an option. SFML may create internal textures padded with white pixels when the GPU doesn't support non-power-of-two dimensions.

Ah, true.
Forgot this since I use only power-of-two textures.

Anyhow, I'd go with that new interface, just with a note that it has this power-of-two requirement.
Though, it would be quite dangerous for people who didn't read documentation...

Hmm... You do have a point


Btw, did I mention that when I call Image.SetSmooth(false) it did fix this problem.

[Laurent]

Anyhow, I'd go with that new interface, just with a note that it has this power-of-two requirement.

From the beginner's point of view:
"What the hell is this 'power-of-two' requirement?? How do I know if my graphics card support it? How can I make sure that people running my game support it as well? ... ok, I'd better ignore this stupid option and copy-paste the sprites I want to repeat"

Btw, did I mention that when I call Image.SetSmooth(false) it did fix this problem.

No, but I did mention it

[Jaenis]

Anyhow, I'd go with that new interface, just with a note that it has this power-of-two requirement.

From the beginner's point of view:

Yep, it is not beginner friendly. But would be advanced user friendly.
Though advanced user can always change that with a OpenGL call.

Btw, did I mention that when I call Image.SetSmooth(false) it did fix this problem.

No, but I did mention it

Hehe, I meant that did I confirm that it did work  :wink:

[Kalith]

I feel bad resurrecting an old thread like this, but I wanted to express my opinion.

I agree the whole "power of two" thing might be a little confusing to beginners, but I don't think this is a good reason to prevent those who understand it from using GL_REPEAT.
If you strictly follow this guiding rule, I bet there wouldn't be much left of SFML Everything is confusing to the beginner. That's why we have docs and forums!

Anyway, I guess it can be changed manually after creating the sf::Image with a few OpenGL calls, but the main reason I use SFML is that I actually don't know OpenGL... (and honestly : I don't want to)
Of course I can read the docs, and I made the change myself in SFML code, but that's not handy.

As Laurent said, repeating the sf::Sprite works too, but it sends N times more vertices to the graphics card (N being the number of time you need to repeat the image), and is rather painful to code :
With GL_REPEAT :

Code: [Select]

// "image" is a 64x64 texture
sf::Sprite sprite(image);
sprite.SetSubRect(sf::IntRect(0, 0, 256, 64));
window.Draw(sprite);
... without :

Code: [Select]

// "image" is a 64x64 texture
sf::Sprite sprite(image);
for (int i = 0; i < 4; ++i)
{
    sprite.Move(64, 0);
    window.Draw(sprite);
}

I don't know for you, but I made my choice.
As I don't use any non-po2 textures, there is no drawback in setting it as the default value, but I understand it might go differently for others.
As Jaenis suggested, I think the best trade-off would be an Image::SetRepeat() function, with GL_CLAMP as a default value.

But that's just my opinion.