Re: Re: Complex Surfaces patch

Marius Grigoriu <[email protected]> Wed, 18 Aug 2004 12:26:51 -0400
Newsgroups gmane.comp.emulators.winex.devel
Message-ID <[email protected]>
Ove Kaaven wrote:

>tor, 12.08.2004 kl. 06.39 skrev Marius Grigoriu:
>  
>
>>Here is the new patch that follows the windows behavior. X11 licensed.
>>    
>>
>
>Here are my concerns with the patch:
>
>* Indentation is wrong.
>* You're using log(). I wouldn't feel comfortable using floating point
>for computing something as exact as mipmap level counts. We use integer
>techniques elsewhere, e.g. dlls/d3d8/texture.c.
>* The role of DDSCAPS_COMPLEX is not well-proven. It is my belief that
>this flag is necessary to create a mipmap chain, but has nothing to do
>with the mipmap count. Perhaps you could check that, i.e., see if you
>can create a mipmap chain *with* DDSD_MIPMAPCOUNT and a count, but
>without DDSCAPS_COMPLEX... if I've guessed right, this should fail, and
>if so, there's no reason to check for DDSCAPS_COMPLEX like this.
>  
>
My memory didn't serve me well when I booted from Windows back to Linux 
so I ran my test app again and played with the flags and wrote down the 
results.

The DDSCAPS_MIPMAP flag was left on.
Without complex and no mipmap count, the length of the chain is 1.
Without complex and a mipmap count, DDERR_INVALIDCAPS
With complex, and a mipmap count, the length of the chain is the number 
given by the count.
With complex and no mipmap count, the length is based on initial size.
With complex and a mipmap count less than 1, E_INVALIDARG

So, COMPLEX is required to create any additional attached surfaces. 
However, the check for COMPLEX in my patch is still necessary. The check 
tests if COMPLEX is set and at the same time, there is no mipmap count. 
Only then the the size of the texture determins the mipmap count. If I 
didnt check for complex, then we would end up creating additional mipmap 
levels even if the surface is not set to be a complex surface.

As for the other points, I had a feeling the log wouldn't be 
appreciated, so I can just change it to a loop and divide by 2. 
Indentation shouldn't be much of a concern, thats easy :)

About returning various errors based on invalid combinations of settings 
in the caps, do you guys care about that? I didn't see any checks like 
that in theses functions so I kept them out of my patch. Should I add 
them so that it returns errors as described above?

Marius