[PATCH] DrawDibDraw flag handling

[Peter Riocreux]

I got a lot of messages from this function about a particular flag
(DDF_BACKGROUNDPAL) not being handled, so I tried to handle it.

The MSDN docs are at
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/multimed/htm/_win32_drawdibdraw.asp

I am coming to this completely cold, so I may be completely off target
here, so if anyone can comment on whether I have done this right. I
suspect that there needs to be explicit support for DDF_SAME_HDC, but
I didn't really understand what it was wanting ....

(Patch is against CVS from this AM (GMT)

Index: dlls/msvideo/drawdib.c
===================================================================
RCS file: /home/wine/wine/dlls/msvideo/drawdib.c,v
retrieving revision 1.21
diff -u -r1.21 drawdib.c
--- dlls/msvideo/drawdib.c      3 Jun 2004 23:19:51 -0000       1.21
+++ dlls/msvideo/drawdib.c      14 Oct 2004 14:46:47 -0000
@@ -311,7 +311,7 @@
     whdd = MSVIDEO_GetHddPtr(hdd);
     if (!whdd) return FALSE;

-    if (wFlags & ~(DDF_SAME_HDC | DDF_SAME_DRAW | DDF_NOTKEYFRAME | DDF_UPDATE | DDF_DONTDRAW))
+    if (wFlags & ~(DDF_SAME_HDC | DDF_SAME_DRAW | DDF_NOTKEYFRAME | DDF_UPDATE | DDF_DONTDRAW | DDF_BACKGROUNDPAL))
         FIXME("wFlags == 0x%08lx not handled\n", (DWORD)wFlags);

if (!lpBits)
@@ -362,7 +362,10 @@
         }
     }
     if (!(wFlags & DDF_DONTDRAW) && whdd->hpal)
-        SelectPalette(hdc, whdd->hpal, FALSE);
+      if ((wFlags & DDF_BACKGROUNDPAL) && ! (wFlags & DDF_SAME_HDC))
+       SelectPalette(hdc, whdd->hpal, TRUE);
+      else
+       SelectPalette(hdc, whdd->hpal, FALSE);

if (!(StretchBlt(whdd->hdc, xDst, yDst, dxDst, dyDst, whdd->hMemDC, xSrc, ySrc, dxSrc, dySrc, SRCCOPY)))
         ret = FALSE;

[Andreas Mohr]

Hi,

On Thu, Oct 14, 2004 at 03:53:41PM +0100, Peter Riocreux wrote:
>  if (!(wFlags & DDF_DONTDRAW) && whdd->hpal)
> -
>    SelectPalette(hdc, whdd->hpal, FALSE);
> -
>  if ((wFlags & DDF_BACKGROUNDPAL) && ! (wFlags & DDF_SAME_HDC))
> -
>   SelectPalette(hdc, whdd->hpal, TRUE);
> -
>  else
> -
>   SelectPalette(hdc, whdd->hpal, FALSE);

This is rather non-obvious if handling, I'm surprised that the compiler
doesn't warn about it, or does it?

Could you add proper braces there?

Thanks!

Andreas Mohr

[Peter Riocreux]

Andreas Mohr  writes:
> This is rather non-obvious if handling, I'm surprised that the compiler
> doesn't warn about it, or does it?

It did, but I was making from the top level and it got lost amongtsh
the other messages.
> Could you add proper braces there?

Fixed up patch below.

Does the patch look sane otherwise? The thing that caused the warnings
still works as before, but doesn't emit those warning now, but I offer
no other claim to correctness.

Peter

Index: dlls/msvideo/drawdib.c
===================================================================
RCS file: /home/wine/wine/dlls/msvideo/drawdib.c,v
retrieving revision 1.21
diff -u -r1.21 drawdib.c
--- dlls/msvideo/drawdib.c      3 Jun 2004 23:19:51 -0000       1.21
+++ dlls/msvideo/drawdib.c      14 Oct 2004 15:23:17 -0000
@@ -311,7 +311,7 @@
     whdd = MSVIDEO_GetHddPtr(hdd);
     if (!whdd) return FALSE;

-    if (wFlags & ~(DDF_SAME_HDC | DDF_SAME_DRAW | DDF_NOTKEYFRAME | DDF_UPDATE | DDF_DONTDRAW))
+    if (wFlags & ~(DDF_SAME_HDC | DDF_SAME_DRAW | DDF_NOTKEYFRAME | DDF_UPDATE | DDF_DONTDRAW | DDF_BACKGROUNDPAL))
         FIXME("wFlags == 0x%08lx not handled\n", (DWORD)wFlags);

if (!lpBits)
@@ -362,7 +362,12 @@
         }
     }
     if (!(wFlags & DDF_DONTDRAW) && whdd->hpal)
-        SelectPalette(hdc, whdd->hpal, FALSE);
+    {
+        if ((wFlags & DDF_BACKGROUNDPAL) && ! (wFlags & DDF_SAME_HDC))
+         SelectPalette(hdc, whdd->hpal, TRUE);
+        else
+         SelectPalette(hdc, whdd->hpal, FALSE);
+    }

if (!(StretchBlt(whdd->hdc, xDst, yDst, dxDst, dyDst, whdd->hMemDC, xSrc, ySrc, dxSrc, dySrc, SRCCOPY)))
         ret = FALSE;

[Dimitrie O. Paun]

On Thu, Oct 14, 2004 at 05:21:41PM +0200, Andreas Mohr wrote:
> Hi,
>
> On Thu, Oct 14, 2004 at 03:53:41PM +0100, Peter Riocreux wrote:
> >  if (!(wFlags & DDF_DONTDRAW) && whdd->hpal)
> > -
> >    SelectPalette(hdc, whdd->hpal, FALSE);
> > -
> >  if ((wFlags & DDF_BACKGROUNDPAL) && ! (wFlags & DDF_SAME_HDC))
> > -
> >   SelectPalette(hdc, whdd->hpal, TRUE);
> > -
> >  else
> > -
> >   SelectPalette(hdc, whdd->hpal, FALSE);
>
> This is rather non-obvious if handling, I'm surprised that the compiler
> doesn't warn about it, or does it?
>
> Could you add proper braces there?

Better yet, what about we just do:
    +       SelectPalette(hdc, whdd->hpal, (wFlags & DDF_BACKGROUNDPAL) && ! (wFlags & DDF_SAME_HDC));

[Peter Riocreux]

"Dimitrie O. Paun"  writes:
> On Thu, Oct 14, 2004 at 05:21:41PM +0200, Andreas Mohr wrote:
> > Hi,
> >
> > On Thu, Oct 14, 2004 at 03:53:41PM +0100, Peter Riocreux wrote:
> > >  if (!(wFlags & DDF_DONTDRAW) && whdd->hpal)
> > > -
> > >    SelectPalette(hdc, whdd->hpal, FALSE);
> > > -
> > >  if ((wFlags & DDF_BACKGROUNDPAL) && ! (wFlags & DDF_SAME_HDC))
> > > -
> > >   SelectPalette(hdc, whdd->hpal, TRUE);
> > > -
> > >  else
> > > -
> > >   SelectPalette(hdc, whdd->hpal, FALSE);
> >
> > This is rather non-obvious if handling, I'm surprised that the compiler
> > doesn't warn about it, or does it?
> >
> > Could you add proper braces there?
>
> Better yet, what about we just do:
> -
>   SelectPalette(hdc, whdd->hpal, (wFlags & DDF_BACKGROUNDPAL) && ! (wFlags & DDF_SAME_HDC));

I wrote it that way because it looked from the docs like
(wFlags & DDF_SAME_HDC) meant something else, different had to be done
with the palette so I wrote it to be expanded further. If that is not
the case then sure - it was what I started with ...

Peter