View/RenderWindow API change [SVN] (issues + solution)

[Julien_v42]

I just updated my SFML from SVN sources, I had a version from April 05th before (which is before the API change in sf::View).
I had to change my view setup from view.Rect = ... to view.SetCenter / view.SetHalfSize to make it compile. And then, all my SFML sprites and text are gone.

I was gonna scream in this forum, but after some trials, I found if I submitted my View each frame, instead of once, I saw the sprites behind my OpenGL stuff. So I was starting to blame RenderWindow for losing its view, and SFML for losing its Z.

Then, I found my mistake. The method RenderWindow::SetView was previously SetView(View *view), and now SetView(const View &view).

Previous (working) code:

Code: [Select]

void Game::SetupStuff()
{
    sf::View view;
    view.Rect = sf::FloatRect(0, 0, viewWidth, viewHeight);
    m_RenderWindow->SetView(&view);
}

New (working) code:

Code: [Select]

void Game::SetupStuff()
{
    const sf::FloatRect rect(0, 0, viewWidth, viewHeight);
    sf::View *view = new sf::View(rect);
    m_RenderWindow->SetView(*view);
}
I should change it to avoid allocation by having Game hold a sf::View member, but that's the idea: you have to keep the View instance somewhere, do not use a local variable.

Hope this helps someone

Oh, and is there a reason not to provide a View::SetRect(const FloatRect &newRect) method? That would be nice for me, as 1) I compute width and height and not halves 2) my view starts at 0, 0 and finish at width, height, 3) that would make the setup one single call, best readability .

[zarka]

You do realize that you leak memory every time you call SetupStuff and what i understand you do this every frame ? never ever make a new without  adding a delete. Especialy if you plan to use the code in a example manner.

here is some better code that does the exact same thing as the code above but does not leak memory!

Code: [Select]


void Game::SetupStuff()
{
    const sf::FloatRect rect(0, 0, viewWidth, viewHeight);
    static sf::View view;
    view.Rect = rect;
    m_RenderWindow->SetView(view);
}


[Laurent]

If you want to use a single view, just use the default one of the window, the GetDefaultView function returns a non-const reference to the view owned by the window.

Oh, and is there a reason not to provide a View::SetRect(const FloatRect &newRect) method? That would be nice for me, as 1) I compute width and height and not halves 2) my view starts at 0, 0 and finish at width, height, 3) that would make the setup one single call, best readability

Then you just need two calls, as the center and half-size of your view are both exactly (width / 2, height / 2)

[Julien_v42]

@Laurent: doh, didn't see that. Saves one View instance

@zarka: I am not doing this each frame! I didn't make it clear, but submitting the view each frame was an attempted workaround for my bug. This is an initial setup. Of course I wouldn't want to leak that memory. Using new was a quick fix to find that was a scope issue, but I was only 'leaking' one instance over the course of the program.


One thing I forgot to mention in my initial post: there was another reason for the second bug (SFML showing behind OpenGL), which is RenderWindow change in default behavior from keeping GL states to not keeping them, adding    m_RenderWindow->PreserveOpenGLStates(true) to my init function solved that.

[zarka]

@zarka: I am not doing this each frame! I didn't make it clear, but submitting the view each frame was an attempted workaround for my bug. This is an initial setup. Of course I wouldn't want to leak that memory. Using new was a quick fix to find that was a scope issue, but I was only 'leaking' one instance over the course of the program.

which is still way to much

but as you can see in my code, if you whant a "local" object to have a lifetime for the whole program duration use a static !

[workmad3]

@zarka: I am not doing this each frame! I didn't make it clear, but submitting the view each frame was an attempted workaround for my bug. This is an initial setup. Of course I wouldn't want to leak that memory. Using new was a quick fix to find that was a scope issue, but I was only 'leaking' one instance over the course of the program.

which is still way to much

but as you can see in my code, if you whant a "local" object to have a lifetime for the whole program duration use a static !

As long as you don't have any concerns about multi-threading anyway. Although considering the ease with which SFML makes creating threaded programs that should be a concern and you should ensure the static variable is accessed in a thread-safe fashion

[Aszarsha]

As long as you don't have any concerns about multi-threading anyway. Although considering the ease with which SFML makes creating threaded programs that should be a concern and you should ensure the static variable is accessed in a thread-safe fashion

You're perfectly right... But I don't think one will make the rendering (which only read view information) part in parallel with the calculation part (which write view information)... It should be ok as long as only reads are theaded.

[workmad3]

That's fine as long as only you will use the code. If you are embedding this into a game engine that you are considering releasing then the relevant idea is 'make no assumptions' as someone, somewhere will do something stupid (or possibly inspired in other circumstances) and it will break because you assumed that something wouldn't be accessed in multiple threads.

Admittedly that is unlikely in this case so not really relevant

[Julien_v42]

Regardless of thread safety, the correct implementation for my case is as follows:

Code: [Select]

void Game::SetupViewOnceAndForAll()
{
    sf::View &view = m_RenderWindow->GetDefaultView();
    const float halfWidth = m_ViewWidth / 2.0f;
    const float halfHeight = m_ViewHeight / 2.0f;
    view.SetCenter(halfWidth, halfHeight);
    view.SetHalfSize(halfWidth, halfHeight);
}