[ros-dev] Gdi function Polygon

[Timo Kreuzer]

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;
}