Hi,

Recently I have started reading Feng Yuan - Windows Graphics Programming Win32 GDI and found that this is a very interesting part of Windows / ReactOS I'd like to take a closer look to.
I started to have a look at this when I noticed 2 things, that made my desk.cpl don't work correctly in ROS:

1.) SetDCPenColor is not implemented (I am currently working on a usermode implemenatation, I have finished GetDCPenColor, GetDCBrushColor is the same, Set... will follow soon.)

2.) Polygon() didn't work as expected. I used it to draw a tiny triangle on a button, but it doesn't look correct. I first thought it was because the triangle was simply too small (didn't show the lowest pixel), but it was because it is redirected to NtGdiPolygon. (instead of calling NtGdiPolyPolyDraw, wich is the way it works in windows. NtGdiPolygon doesn't seem to exist in Windows, so it seems it's currently only a Kernelmode implementation of Polygon and NtGdiPolyPolyDraw seems not to be implemented in ROS yet).
Polygon is exspected to automatically close the polygon, but NtGdiPolygon doesn't do this. I will have a closer look on this later and hope I can fix this. My questions:
Is there a reason it doesn't do it? Should the closing of the Polygon (simply by adding the first coordinates at the end?) be done in the usermode function (wich is not existing yet) or in the current implementation in win32k?

Looking at the gdi code I found out that some things (handle validation for example) seem to be different than in windows. F.e. the reuse counter seems to be ignored. I have written a small GdiTest app to analyse the diffences between Win and ROS (General app is ready, I will write test functions soon, maybe tomorrow) . I'll start by writing some test cases, that I think might fail on ROS. If there's someone who likes to help me writing tests I'd really appreaciate that, just let me know.

Then there's another thing that I thought about: Windows has one list of gdiobjects shared between kernel and all processes, although handles are only valid for the process that has created them (only Stockobjects are global). That seems to be a bad idea, I didn't try it yet, but what if a usermode app zeroes out the complete gdi handle table? I guess Windows would crash. And there might be more dangoerous risks, because of that. So why is the table not implemented locally?

Some thoughts:
Global handle list:
 + less memory needed
 - big chance of crashing windows by usermode apps

Local handle list
 - more memory needed
 + better chance of paging the memory (global list would never be paged as it is always in use)
 + no chance for a user app to crash windows or crash other apps by messing up the handle table
 + no more need to veryfy the process member in the handle table entry

A shared memory section seems to be a real risky thing. It's big enough to put binary data there and it might be possible to make another process with higher privaliges execute that code.I first thought of the DC's AbortProc, but that is probaly handled by the kernel to not exceed user limits(?).

Implementing the handle table locally would not beak any APIs. Only some system apps that use the GdiQueryTable and directly use the table would experience a slightly different behaviour: Only locally created handles would be accessable. Would there be a problem doing that?

At last my usermode implementation of GetDCPenColor, I would like some feedback:

#define GDI_HANDLE_UPPER_MASK 0xffff0000
#define GDI_HANDLE_GET_UPPER(h)     \
    (((ULONG_PTR)(h)) & GDI_HANDLE_UPPER_MASK)

COLORREF
GetDCPenColor(HDC hDC)
{
   PGDI_TABLE_ENTRY pEntry = GdiHandleTable + GDI_HANDLE_GET_INDEX(hGdiObj);
   if(pEntry->KernelData != NULL)
   {
      ULONG Upper = GDI_HANDLE_GET_UPPER(hGdiObj)
      if ( Upper == GDI_HANDLE_GET_UPPER(pEntry->Type)
           && GDI_HANDLE_GET_TYPE(Upper) == GDI_OBJECT_TYPE_DC )
      {
         if((HANDLE)((ULONG_PTR)pEntry->ProcessId & ~0x1) == CurrentProcessId)
         {
            DC_ATTR* pDCData = (DC_ATTR*)pEntry->UserData
            if(pDCData)
            {
               return pDCData->crPenClr;
            }
         }
      }
   }
   return CLR_INVALID;
}