[Kstars-devel] Re: Planet textures with OpenGL

[Akarsh Simha]

On Sat, Apr 09, 2011 at 02:30:24PM +0400, Alexey Khudyakov wrote:
> On 09.04.2011 05:45, Akarsh Simha wrote:
> >>IMHO the best way to deal with the problem is to aquire texture ID
> >>when they are used for first time. So they will be generated after
> >>transfer of ownership.
> >
> >There might be a better way. Looking at the code,
> >QContextPrivate::bindTexture() is supposed to check if the texture
> >already has a texture ID and then bind it. It does maintain a cache of
> >textures. If my understanding is correct, ideally, it should
> >invalidate that cache as well when the context is reset. So maybe it
> >is best to solve this in QGLContextPrivate itself. I need to ask on
> >#qt or something and I plan to do it this weekend.
> >
> Indeed. QGLContext states:
> 
> The texture that is generated is cached, so multiple calls to
> bindTexture() with the same QImage will return the same texture id.
> 
> So you propose to avoid caching texture IDs at all and rely on Qt to
> do that, am I right?

Yes, that's what I mean. But then, that doesn't seem to work
out-of-the-box if I did my experiments right. I suspect that
QGLContext does not clean its texture cache when it is reset, but I
need to check that. Need to do some asking around.

> >>Maybe TextureManager should not own context at all and have it
> >>passed in as parameter. This way we can avoid transfering of context
> >>at all.
> >
> >That sounds good to me! We have a Q_ASSERT( m_context ) whenever we
> >try to bind a texture and we can have a TextureManager::setContext(
> >QGLContext * ), and we can pass the context of the QGLWidget.
> >
> I think setContext is bad idea. TextureManager do not own context so
> it should not hold pointer to it. If context is passed as parameter
> than it's not possible to have incoherence when manager holds one
> context and caller expect another.

Hmm, okay.

So you plan to instead call something like:

TextureManager::getTexture( "texture", m_widget->getContext() );

But somehow that doesn't look very good to me...


> Also I think Texture class isn't really required. It and
> TextureManager rely on implementation of each other. There is no
> point in separating these classes.
> 
> Following API will do the trick I assume. Anyway texture cannot do
> anything but return QImage and bind texture. Also there is no need
> to make TextureManager a Q_OBJECT.

I don't see any reasons why TextureManager is a QObject either. 
@Harry: Did you have any reasons to make TextureManager a QObject.

> class TextureManager {
> public:
>   // Return image and tries load it if not already loaded
>   static const QImage& getImage(const QString& name);
>   // Binds texture
>   static bool bindTexture(const QString& name )
>   {
>     const QImage& = getImage(name);
>     // bind texture here
>   }
> ...
> }

I think this is a good idea. Yes, Texture seems to be redundant, and
texture manager can keep track of it.

So you would have a hash: QHash<QString, QImage> m_Textures; and you
could then either

bindTexture( image );

for GL, and

return &image;

for QPainter.

Is that what you mean? Sounds good to me. Does Harry have any
objections?

Regards
Akarsh