285054 - ConvertDDBtoDIB violates windows APIs

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Win32, defect)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

quoting
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/gdi/bitmaps_7gms.asp

"hbmp
    [in] Handle to the bitmap. This must be a compatible bitmap (DDB)."

"The bitmap identified by the hbmp parameter must not be selected into a device
context when the application calls this function."

This function does select the bitmap into a DC:
1661       oldbits = (HBITMAP)::SelectObject(memPrDC,mHBitmap);

Also, in many cases mHBitmap seems to be a DIB, as created by CreateDIBSection:
http://lxr.mozilla.org/seamonkey/source/gfx/src/windows/nsImageWin.cpp#1498

Comment 1

[ArronM (:paper)]

With regards to mHBitmap being a DIBSection, we really don't need to call
ConvertDDBtoDIB at all, if we didn't null out mImageBits and mImageSize.  I
think that should be another bug though.

As for the bug in question, the SelectObject has been in the code since May,
1999.  But you're right, it's incorrect according to the spec.  Looks like the
two SelectObject calls and the oldbits variable can be removed.

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

if we didn't delete and null out mImageBits, we'd take up twice the memory for
the images, wouldn't we?

Comment 3

[ArronM (:paper)]

When the DIB is created, we copy mImageBits into the "bits" pointer that the OS
gave us, and then clear mImageBits.  What I meant to say in Comment 1 was that
we can "delete [] mImageBits; mImageBits = bits;", leaving only 1 copy.

Comment 4

[ArronM (:paper)]

Attachment: Don't select bitmap into HDC

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 178215 [details] [diff] [review]
Don't select bitmap into HDC

this still passes a DIB to GetDIBits sometimes, but ok...

Comment 6

[ArronM (:paper)]

(In reply to comment #5)
> (From update of attachment 178215 [details] [diff] [review] [edit])
> this still passes a DIB to GetDIBits sometimes, but ok...

one bug at a time :)

Comment 7

[ArronM (:paper)]

Checking in gfx/src/windows/nsImageWin.cpp;
/cvsroot/mozilla/gfx/src/windows/nsImageWin.cpp,v  <--  nsImageWin.cpp
new revision: 3.144; previous revision: 3.143
done

Comment 8

[Asa Dotzler [:asa]]

Has this not actually landed? Why is it marked Fixed?

Comment 9

[ArronM (:paper)]

Comment on attachment 178215 [details] [diff] [review]
Don't select bitmap into HDC

sorry, I flagged approval1.8b2 request on the wrong bug.