Re: patch for error handling after driver callback calls.

Hi Alexandre,

I understand your idea and it makes perfect sense. Maybe the description of my
patch wasn't clear enough. And maybe I shouldn't have changed SetTextColor and
SetBkColor (that were actually the only callbacks that were well implemented on
both the driver side and the the GDI). But if you look at most of the other
callback implementations in the EMF and WMF drivers (SetTextAlign for example),
you'll see that they only return TRUE or FALSE (the return value from
WriteRecord) and not what the corresponding API function should return. So, the
code that handles the callback return value doesn't interpret the return code
correctly.

There's clearly something wrong with some callbacks though. We just need to
agree on a convention and respect it in all driver callbacks... So what if I
regenerate a patch that looks like this in the case of SetTextAlign. Would you
accept such a patch?

----------------------------------------------------------------------------------

--- wine-20030911/objects/dc.c     Mon Oct  6 16:46:07 2003
+++ wine/objects/dc.c    Fri Oct 10 11:39:43 2003
@@ -904,13 +904,17 @@
 {
     UINT prevAlign;
     DC *dc = DC_GetDCPtr( hdc );
-    if (!dc) return 0x0;
-    if (dc->funcs->pSetTextAlign)
-        prevAlign = dc->funcs->pSetTextAlign(dc->physDev, align);
-    else {
-    prevAlign = dc->textAlign;
-    dc->textAlign = align;
+    if (!dc) return GDI_ERROR;
+    prevAlign = dc->textAlign;
+    if (dc->funcs->pSetTextAlign) {
+    align = dc->funcs->pSetTextAlign(dc->physDev, align);
+    if (align == GDI_ERROR) {
+        /* don't change it */
+        align = prevAlign;
+        prevAlign = GDI_ERROR;
+    }
     }
+    dc->textAlign = align;
     GDI_ReleaseObj( hdc );
     return prevAlign;
 }

--- wine-20030911/dlls/gdi/enhmfdrv/dc.c        Wed Oct  8 10:58:22 2003
+++ wine/dlls/gdi/enhmfdrv/dc.c Fri Oct 10 11:45:50 2003
@@ -46,7 +46,8 @@
     emr.emr.iType = EMR_SETTEXTALIGN;
     emr.emr.nSize = sizeof(emr);
     emr.iMode = align;
-    return EMFDRV_WriteRecord( dev, &emr.emr );
+    return EMFDRV_WriteRecord( dev, &emr.emr ))
+        ? ((EMFDRV_PDEVICE*)dev)->dc->textAlign : GDI_ERROR;
 }

 INT EMFDRV_SetBkMode( PHYSDEV dev, INT mode )

----------------------------------------------------------------------------------

Thanks.

Dave





Alexandre Julliard <julliard(a)winehq.com> on 10/09/2003 07:37:42 PM

To:   Dave Belanger/CSI(a)CSIDomain
cc:   wine-devel(a)winehq.com

Subject:  Re: patch for error handling after driver callback calls.

Dave_Belanger(a)cimmetry.com writes:
> (See attached file: patch_callbacks_return.txt)

The idea is that the DC interface functions should behave just like
the corresponding APIs, so if the API returns the old value the DC
function should do that too. This way it guarantees that we can
implement the full behavior of the API in the driver.

--
Alexandre Julliard
julliard(a)winehq.com

patch for error handling after driver callback calls.

[Alexandre Julliard]

Dave_Belanger(a)cimmetry.com writes:
> I understand your idea and it makes perfect sense. Maybe the description of my
> patch wasn't clear enough. And maybe I shouldn't have changed SetTextColor and
> SetBkColor (that were actually the only callbacks that were well implemented on
> both the driver side and the the GDI). But if you look at most of the other
> callback implementations in the EMF and WMF drivers (SetTextAlign for example),
> you'll see that they only return TRUE or FALSE (the return value from
> WriteRecord) and not what the corresponding API function should return. So, the
> code that handles the callback return value doesn't interpret the return code
> correctly.

Actually it's the driver that isn't returning the right thing, AFAICS
the caller is doing the right thing, at least for SetTextAlign.
Though it seems in some cases, like SetTextColor, it expects the
driver to return the new color instead of the old one; that should
probably be fixed too. The driver interface should really follow the
API as closely as possible.

-- 
Alexandre Julliard
julliard(a)winehq.com