Implemented support for mipmap generation

[binary1248]

This is the GL_EXT_framebuffer_object variant that relies on explicit generation by the user rather than implicit generation every time the base level image is modified.

Proper support for sf::RenderTexture has also been implemented.

Supersedes #498, implements #123.

[LaurentGomila]

Looks good overall.

Some remarks:

• Do we really need to expose a hasMipMap() function? I can't see any useful use of such a function (regarding how sf::RenderTexture uses it: the check could be done inside invalidateMipmap() so the mipmap state could be kept private inside sf::Texture)?
• Do we really want to remove manual texture management from the OpenGL example? The point of this example is to show interoperability between SFML and OpenGL.
• Can't we invalidate the mipmaps inside sf::RenderTexture::display(), to avoid making the draw function virtual?
• Shouldn't we invalidate the mipmap automatically in loadFromXxx and update?
• I would also explain what mipmap is and in which circumstances it is used by the graphics card, in the function and/or class documentation.

[binary1248]

Do we really need to expose a hasMipMap() function? I can't see any useful use of such a function (regarding how sf::RenderTexture uses it: the check could be done inside invalidateMipmap() so the mipmap state could be kept private inside sf::Texture)?

While hasMipmap() is not really necessary, it might be convenient for the user if their architecture requires them to be able to tell if mipmaps need to be regenerated or not. If we don't track this state, they will have to themselves, and I assume it is simpler for us to do directly in sf::Texture than it is for them to implement in some auxiliary class.

Also, invalidateMipmap() is a private function at the moment, only meant to be used by RenderTexture (which is already a friend class). I don't see any reason to make it public if we already make sure any modification by the user results in proper invalidation.

Do we really want to remove manual texture management from the OpenGL example? The point of this example is to show interoperability between SFML and OpenGL.

From what I can tell (you can see this in the comment which I forgot to change when I removed GLU), the only reason for the manual management in that example was because we wanted to use GLU to generate the mipmap. Since GLU got removed, and we can even generate the mipmap ourselves now, there is no point to manually manage texture creation. And besides... this is a better example of interoperability between SFML and OpenGL if you ask me. 😉

Can't we invalidate the mipmaps inside sf::RenderTexture::display(), to avoid making the draw function virtual?

Well... it depends on what we tell users who don't bother calling sf::RenderTexture::display() after they are done drawing. The main problem is that the behaviour varies based on what implementation is used. When using the default implementation, the texture will really only be updated once display() is called and not before. When using the FBO implementation, calling display() is not necessary if you are lucky since the texture is directly modified in every draw. You can call it an error to not call display() even if it isn't necessary in this case, but invalidating in draw() ensures consistent behaviour regardless of which implementation is used.

What makes it even worse is that the current mipmap implementation doesn't even work unless the FBO implementation is available in the first place, so the case where it would actually make sense (the default implementation) doesn't even happen in practice.

Shouldn't we invalidate the mipmap automatically in loadFromXxx and update?

It does get invalidated. I just omitted the additional texture saving etc. that is present in invalidateMipmap() and inlined the 2 lines that actually perform the "invalidation". Like I said above, invalidateMipmap() is only meant for RenderTexture, that's why we need to do a bit more in there.

[LaurentGomila]

While hasMipmap() is not really necessary, it might be convenient for the user if their architecture requires them to be able to tell if mipmaps need to be regenerated or not. If we don't track this state, they will have to themselves, and I assume it is simpler for us to do directly in sf::Texture than it is for them to implement in some auxiliary class.

Not sure. In most scenarios, users will know exactly when to regenerate mipmaps without tracking a state (after the call to load or update, or every frame). I'd really prefer to go with the minimal API first, and add this kind of function (which is pure convenience) later if really needed.

this is a better example of interoperability between SFML and OpenGL if you ask me

I thought about it again and reached the same conclusion. I even knew that you would answer that 😛

Well... it depends on what we tell users who don't bother calling sf::RenderTexture::display() after they are done drawing

So far, we have always enforced the call to display() on render textures. Not calling it could be considered a UB. I don't like half-defined states, or assertions that are verified only under some environments and not some others. The API is well-defined, the fact that it works sometimes without it is only a side-effect. This also leaves us more room for implementation -- like in this very specific case. We don't break anything if we do it in display(), so I don't see why we shouldn't do it. Given the "ugly" modification required to the API, I think it's worth it.

It does get invalidated

Sorry, I missed that in the diff.

And don't forget my last point, I guess I added it after you read my post:

I would also explain what mipmap is and in which circumstances it is used by the graphics card, in the function and/or class documentation.

[binary1248]

The issues should be fixed now.

[LaurentGomila]

👍

Does this need more testing on OpenGL ES platforms?

[binary1248]

Rebased and added missing define for OpenGL ES platforms.

Could use some OpenGL ES testing. @MarioLiebisch, @BlueCobold

[BlueCobold]

Will see if I can find some time this weekend to check for iOS.

[binary1248]

Bump.

[TankOs]

Working flawlessly on Arch Linux. 👍 🐈

[eXpl0it3r]

@MarioLiebisch, @BlueCobold Any testing on this?

[eXpl0it3r]

Since iOS/Android are still experimental, should we proceed to and merge this?

[BlueCobold]

Is there a minimal sample to test this? Then it would be far easier for me to check if it's working - basically just compile the libs and test it.

[eXpl0it3r]

The OpenGL example uses the mipmaps, so you could just built these, or does that not work for iOS?

[BlueCobold]

I'll check.

[eXpl0it3r]

Any results so far @BlueCobold?

What about @MarioLiebisch?

[eXpl0it3r]

@binary1248 Could you rebase the PR?

Also could we potentially move on without proper OpenGL ES testing?

[binary1248]

Done.

[eXpl0it3r]

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.