Signed-off-by: Jeff Smith whydoubt@gmail.com --- Testing revealed that SetBkMode() does not treat 'invalid' values as one might assume. Discovered this when working on another issue. Note that fixing this to match Windows simplifies one of the tests for that bug.
dlls/gdi32/dc.c | 12 ++---------- dlls/gdi32/tests/dc.c | 19 ++++++++++++++++++- 2 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/dlls/gdi32/dc.c b/dlls/gdi32/dc.c index 230f1949e8d..0bac9c27d16 100644 --- a/dlls/gdi32/dc.c +++ b/dlls/gdi32/dc.c @@ -1598,20 +1598,12 @@ INT WINAPI SetBkMode( HDC hdc, INT mode ) INT ret = 0; DC *dc;
- if ((mode <= 0) || (mode > BKMODE_LAST)) - { - SetLastError(ERROR_INVALID_PARAMETER); - return 0; - } if ((dc = get_dc_ptr( hdc ))) { PHYSDEV physdev = GET_DC_PHYSDEV( dc, pSetBkMode ); mode = physdev->funcs->pSetBkMode( physdev, mode ); - if (mode) - { - ret = dc->backgroundMode; - dc->backgroundMode = mode; - } + ret = dc->backgroundMode; + dc->backgroundMode = mode; release_dc_ptr( dc ); } return ret; diff --git a/dlls/gdi32/tests/dc.c b/dlls/gdi32/tests/dc.c index b4a89b213d2..ce9fca1653f 100644 --- a/dlls/gdi32/tests/dc.c +++ b/dlls/gdi32/tests/dc.c @@ -40,7 +40,7 @@ static void test_dc_values(void) { HDC hdc = CreateDCA("DISPLAY", NULL, NULL, NULL); COLORREF color; - int extra; + int extra, mode;
ok( hdc != NULL, "CreateDC failed\n" ); color = SetBkColor( hdc, 0x12345678 ); @@ -81,6 +81,23 @@ static void test_dc_values(void) extra = GetTextCharacterExtra( hdc ); ok( extra == 123, "initial extra %d\n", extra );
+ mode = SetBkMode( hdc, TRANSPARENT ); + ok( mode == OPAQUE, "initial mode %08x\n", mode ); + mode = GetBkMode( hdc ); + ok( mode == TRANSPARENT, "wrong mode %08x\n", mode ); + mode = SetBkMode( hdc, OPAQUE ); + ok( mode == TRANSPARENT, "wrong mode %08x\n", mode ); + mode = GetBkMode( hdc ); + ok( mode == OPAQUE, "wrong mode %08x\n", mode ); + mode = SetBkMode( hdc, 0 ); + ok( mode == OPAQUE, "wrong mode %08x\n", mode ); + mode = GetBkMode( hdc ); + ok( mode == 0, "wrong mode %08x\n", mode ); + mode = SetBkMode( hdc, BKMODE_LAST + 1 ); + ok( mode == 0, "wrong mode %08x\n", mode ); + mode = GetBkMode( hdc ); + ok( mode == BKMODE_LAST + 1, "wrong mode %08x\n", mode ); + DeleteDC( hdc ); }
On Tue, Oct 06, 2020 at 11:22:29PM -0500, Jeff Smith wrote:
Signed-off-by: Jeff Smith whydoubt@gmail.com
Testing revealed that SetBkMode() does not treat 'invalid' values as one might assume. Discovered this when working on another issue. Note that fixing this to match Windows simplifies one of the tests for that bug.
Which then begs the question: How should these invalid modes be treated visually?
Since there doesn't appear to be a real application that depends on this, we should keep things as they are.
Huw.
On Wed, Oct 7, 2020 at 2:24 AM Huw Davies huw@codeweavers.com wrote:
On Tue, Oct 06, 2020 at 11:22:29PM -0500, Jeff Smith wrote:
Signed-off-by: Jeff Smith whydoubt@gmail.com
Testing revealed that SetBkMode() does not treat 'invalid' values as one might assume. Discovered this when working on another issue. Note that fixing this to match Windows simplifies one of the tests for that bug.
Which then begs the question: How should these invalid modes be treated visually?
FWIW, testing In Windows points to the default being transparent. That implies that any wine code that checks this value should check for equality/non-equality with OPAQUE to be "safe".
Since there doesn't appear to be a real application that depends on this, we should keep things as they are.
Would be nice (and removes 8 unnecessary lines of code), but no real harm in leaving it as it is.
Thanks, Jeff
On Wed, 7 Oct 2020 at 18:35, Jeff Smith whydoubt@gmail.com wrote:
On Wed, Oct 7, 2020 at 2:24 AM Huw Davies huw@codeweavers.com wrote:
On Tue, Oct 06, 2020 at 11:22:29PM -0500, Jeff Smith wrote:
Signed-off-by: Jeff Smith whydoubt@gmail.com
Testing revealed that SetBkMode() does not treat 'invalid' values as
one
might assume. Discovered this when working on another issue. Note
that
fixing this to match Windows simplifies one of the tests for that bug.
Which then begs the question: How should these invalid modes be treated visually?
FWIW, testing In Windows points to the default being transparent. That implies that any wine code that checks this value should check for equality/non-equality with OPAQUE to be "safe".
What ends up in metafile record for invalid modes?
Since there doesn't appear to be a real application that depends on this, we should keep things as they are.
Would be nice (and removes 8 unnecessary lines of code), but no real harm in leaving it as it is.
Thanks, Jeff
On Wed, Oct 7, 2020 at 10:53 AM Nikolay Sivov bunglehead@gmail.com wrote:
On Wed, 7 Oct 2020 at 18:35, Jeff Smith whydoubt@gmail.com wrote:
On Wed, Oct 7, 2020 at 2:24 AM Huw Davies huw@codeweavers.com wrote:
On Tue, Oct 06, 2020 at 11:22:29PM -0500, Jeff Smith wrote:
Signed-off-by: Jeff Smith whydoubt@gmail.com
Testing revealed that SetBkMode() does not treat 'invalid' values as one might assume. Discovered this when working on another issue. Note that fixing this to match Windows simplifies one of the tests for that bug.
Which then begs the question: How should these invalid modes be treated visually?
FWIW, testing In Windows points to the default being transparent. That implies that any wine code that checks this value should check for equality/non-equality with OPAQUE to be "safe".
What ends up in metafile record for invalid modes?
It contains the unaltered value for both valid and invalid modes, as a little-endian 32-bit signed int of course.
Tested with -1 0 1 2 3 0xdeadbeef.
I tested on wine, with the patch applied, and got matching results.
Since there doesn't appear to be a real application that depends on this, we should keep things as they are.
Would be nice (and removes 8 unnecessary lines of code), but no real harm in leaving it as it is.
Thanks, Jeff
-
Huw Davies -
Jeff Smith -
Nikolay Sivov