[ros-diffs] [jgardou] 48359: [WIN32K] - Rewrite NtGdiGetDIBitsInternal, with WINE as a reference. - Get back DIB Section creation to classic BITMAPINFO. This si the beginnig of the end for all this BITMAPV5INFO stuff. It is horrible, BITMAPCOREINFO->

[James Tabor]

Hi!

BitmapCoreInfo !Should Be! handled in Gdi32 and converted there! The
latest BitmapInfo "must be" handled in win32k. So not get it confused.
James

[Jérôme Gardou]

That's why I began to revert all of this stuff, which is horrible :-)
Regards.
Jérôme.

James Tabor wrote:
>   Hi!
>
>  BitmapCoreInfo !Should Be! handled in Gdi32 and converted there! The
>  latest BitmapInfo "must be" handled in win32k. So not get it confused.
>  James
>
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>
>
>

[Timo Kreuzer]

Jérôme Gardou schrieb:
>   That's why I began to revert all of this stuff,
> which is horrible :-) 
While I completely agree on removing BITMAPCOREHEADER support, I don't
really like other parts.

- Probing the BITMAPINFO and then passing the usermode buffer to the
internal and unprotected function is not enough. The buffer must be copied.
- The BITMAPV5HEADER is only ~120 bytes and only used once per function
call, so putting the safe buffer on the stack is appropriate. The buffer
needs be large enough for V5 anyway, so why not fixup some values and
make it a full V5 header?
- The ProbeAndConvert function was added to simplify the code and move
all code that handles the small differences into one central place
instead of having a bunch of if (size > bla) checks in multiple places
just to check whether I can or can not access some member of the
BITMAPINFO or where to get the color masks from. With the conversion all
of the internal code can work the same simple way assuming a full
BITMAPV5INFO is available.
- When support for BITMAPCOREHEADER gets removed the code become much
less "horrible"

Timo

[Jérôme Gardou]

Timo Kreuzer wrote:
>   Jérôme Gardou schrieb:
>     
> >   That's why I began to revert all of this
> > stuff, which is horrible :-)
> >        
>  While I completely agree on removing BITMAPCOREHEADER support, I don't
>  really like other parts.
>
>  - Probing the BITMAPINFO and then passing the usermode buffer to the
>  internal and unprotected function is not enough. The buffer must be copied.
>      
OK, it's safer this way.
>   - The BITMAPV5HEADER is only ~120 bytes and only used
> once per function
>  call, so putting the safe buffer on the stack is appropriate. The buffer
>  needs be large enough for V5 anyway, so why not fixup some values and
>  make it a full V5 header?
>      
No, all values added by V4/V5 headers are for ICM. If there is only a 
BITMAPINFOHEADER there, then we don't have to care about ICM. If we 
"convert" it to a V5 header, all values will be 0, that's not worth the 
effort.
>   - The ProbeAndConvert function was added to simplify
> the code and move
>  all code that handles the small differences into one central place
>  instead of having a bunch of if (size>  bla) checks in multiple places
>  just to check whether I can or can not access some member of the
>  BITMAPINFO or where to get the color masks from. With the conversion all
>  of the internal code can work the same simple way assuming a full
>  BITMAPV5INFO is available.
>      
The only "effort" to make here is to take care of where the color
buffer 
is : bmi + bmi->bmiHeader.biSize and not bmi->bmiColors. Then check in 
some functions if we have a V4/V5 header to take care of the features 
they have. If we're good enough, we'll just need to call 
DIB_CreateDIBSection, and work with this bitmap created here. That's 
what I did in NtGdiStretchBitmapInternal, NtGdiGetDIBits and 
IntGdiSetDIBits. The major problem is to get the palette created properly.
>   - When support for BITMAPCOREHEADER gets removed the
> code become much
>  less "horrible"
>      
Agree.
>   Timo
>
>      
Jérôme.

[Timo Kreuzer]

Jérôme Gardou wrote:
> >  
> >
> >  - Probing the BITMAPINFO and then passing the usermode buffer to the
> >  internal and unprotected function is not enough. The buffer must be
> >  copied.
> >      
>  OK, it's safer this way. 
It's not a matter of "safe, safer,
safest". It's wrong vs correct.
Accessing the usermode buffer without SEH == unsafe!

> >   - The
> > BITMAPV5HEADER is only ~120 bytes and only used once per function
> >  call, so putting the safe buffer on the stack is appropriate. The buffer
> >  needs be large enough for V5 anyway, so why not fixup some values and
> >  make it a full V5 header?
> >      
>  No, all values added by V4/V5 headers are for ICM. If there is only a
>  BITMAPINFOHEADER there, then we don't have to care about ICM. If we
>  "convert" it to a V5 header, all values will be 0, that's not worth
>  the effort. 
There are also the color masks. In V4/V5 there are dedicated fields
for
these masks, while in the BITMAPINFO they are in the bmiColors array.
If *bV5CSType == *LCS_sRGB or LCS_WINDOWS_COLOR_SPACE, then the other
fields are ignored, so here is no additional effort.

 
>   The only "effort" to make here is to take
> care of where the color
>  buffer is : bmi + bmi->bmiHeader.biSize and not bmi->bmiColors. Then
>  check in some functions if we have a V4/V5 header to take care of the
>  features they have.  
Exactly. And why do you want to multiply this effort? Why do
you want to
handle this everywhere you use a BITMAPINFO instead of in exactly one
place?


Timo