sf::Sprite::SetImage - distorted rendering

[Nexus]

Hello, the following code

Code: [Select]

int main()
{
    sf::RenderWindow window(sf::VideoMode(1000, 800), "SFML test");

    sf::Image image1;
    image1.LoadFromFile("resources/background.jpg");

    sf::Image image2; // smaller than image1
    image2.LoadFromFile("resources/texture.jpg");

    sf::Sprite sprite(image1);
    sprite.SetImage(image2);

    while (window.IsOpened())
    {
        sf::Event event;
        while (window.GetEvent(event))
        {
            if (event.Type == sf::Event::Closed || event.Type == sf::Event::KeyPressed)
                return 0;
        }

        window.Clear();
        window.Draw(sprite);
        window.Display();
    }
}leads to a distorted rendering of image2. The sprite itself is drawn correctly, but besides it, the space of image1 is filled with artifacts. When I create the sprite directly with

Code: [Select]

sf::Sprite sprite(image2);everything is okay. Tell me if you need a screenshot or if I should add the issue to the task tracker.

By the way, the code uses the images of the OpenGL example.

[devlin]

set the adjustToNewSize parameter of SetImage to true instead of the default false?

Seems like intended behaviour to me.

[Nexus]

Thanks. I haven't used SetImage() for a long time and didn't even know about that parameter

Anyway, I'm not sure whether it is a good idea to leave the remaining space "undefined". Maybe it would be better if sub-rects that are bigger than the image itself have clear semantics (an option is also to forbid them, and maybe check with assert).

And I thought about setting the default value of adjustToNewSize to true, but that seems to be unintuitive in some situations, too. For example at a tileset where you only change the texture, but not the tile subrect.

[Tank]

And I thought about setting the default value of adjustToNewSize to true, but that seems to be unintuitive in some situations, too. For example at a tileset where you only change the texture, but not the tile subrect.

There has been a lot of discussion about it, that's why the second parameter for sf::Sprite::SetImage() was added. However I (still) agree with you that the default behaviour should be to use the full space of the new bound image.

The question is what most of the users expect to get when setting a new image. In OpenGL for example it's different, because you explicitly specify texture coordinates that don't change. But for sf::Sprite it's not so obvious, because SetSubRect() is not an elemental feature (design-wise). In my opinion it's more like "I set this image, so I want to see this image."

Also, when using SetImage() the first time, the sub rect is adjusted automagically, but when using it a second time, it isn't, which may also be confusing.

And last but not least the case where you change the image and want to keep the exact sub rect may happen not that much -- I may be wrong, let's hear what others say.

An alternative would be to check if the sub rect has been set manually and then keep it on further SetImage() calls.

[devlin]

But for sf::Sprite it's not so obvious, because SetSubRect() is not an elemental feature (design-wise). In my opinion it's more like "I set this image, so I want to see this image."

I respectfully disagree.

Image as a sprite sheet seem the most natural connection to sf::Image from sf::Sprite - and it's far more maintainable to keep colour- or look-variations (for example: after grabbing a powerup) for the same sprite in different images and swap between them by setting a new image while maintaining subrects for say the current animation.

[Tank]

I understand that, it highly depends on the proper situation. However, the behaviour is still inconsistent in my opinion (has nothing to do with your statement, of course).

[Nexus]

The problem is that the constructor of sf::Sprite looks like a SetImage(), in fact it has the semantics SetImage() and SetSubRect(). I don't know whether adjustToNewSize=true would be more consistent, because then the attributes image and subrect aren't independent anymore. A SetImage() call would imply a SetSubRect() call, which may also be surprising.

Anyway... Laurent, could you imagine to specify the behavior of subrects larger than images in the documentation? In my opinion, the most meaningful option is to simply not allow it, as it is almost always a logic error. Maybe an assertion could support that.

[Laurent]

Anyway... Laurent, could you imagine to specify the behaviour of subrects larger than images in the documentation? In my opinion, the most meaningful option is to simply not allow it, as it is almost always a logic error. Maybe an assertion could support that.

This kind of stuff may change after I rewrite the drawables system, so I prefer to wait until it's done before deciding anything.

[Gibgezr]

But for sf::Sprite it's not so obvious, because SetSubRect() is not an elemental feature (design-wise). In my opinion it's more like "I set this image, so I want to see this image."

I respectfully disagree.

Image as a sprite sheet seem the most natural connection to sf::Image from sf::Sprite - and it's far more maintainable to keep colour- or look-variations (for example: after grabbing a powerup) for the same sprite in different images and swap between them by setting a new image while maintaining subrects for say the current animation.

Hmm, but this may not be a very good idea for a realtime application, as changing Images causes an OpenGL statechange, which is very expensive i.e. slow. Optimally, you would put ALL cells for that sprite on the same image, and just adjust the SubRect for the "powerup" versions. A great way to do that is to lay the sprite cells out in an organized fashion on the Image, for example putting the powere-up versions directly below their "non-power up" counterparts. Then, you adjust the SubRect by calculating it with code that uses a powerup flag, like so:

Code: [Select]


void spriteObject::animate(float timePassed)
{
timeSinceFrameAdvance += timePassed;

if (timeSinceFrameAdvance >= timePerFrame)
{
currentFrame++;
timeSinceFrameAdvance = 0;
//could instead do timeSinceFrameAdvance -= timePerFrame,
//can be a tiny bit smoother under erratic framerates

animate(currentFrame);
}
}

void spriteObject::animate(int frameToGoTo)
{
currentFrame = frameToGoTo;

if (currentFrame > totalFrames)
currentFrame = 0;

// Set the sprite's image to the current frame of animation
theSprite->SetSubRect(sf::IntRect(
currentFrame*cellWidth,
powerup*cellHeight,
cellWidth, cellHeight);
}


You want to design for as few statechanges as possible (they are dreadfully slow in OpenGL), so you try and batch all your drawables by putting them into as few Images as possible, and drawing all of the ones that are sourced from the same Image before moving to ones that are sourced from another Image. Proper design of the spritesheets (Images) will aid the coding significantly, and the only real issues are Image size and the development headaches around "having a lot of art in one asset".

[devlin]

You want to design for as few statechanges as possible (they are dreadfully slow in OpenGL)

I've worked with OpenGL for over a decade now, but thanks for the tips