Re: [Gdiplus 11/15] Implement GdipCreateRegionRect
On Wed, Jul 09, 2008 at 03:34:01AM -0400, Adam Petaccia wrote:
--- dlls/gdiplus/region.c | 28 ++++++++++++++++++++++++---- dlls/gdiplus/tests/region.c | 8 ++++---- 2 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/dlls/gdiplus/region.c b/dlls/gdiplus/region.c index 59e2fa6..de86ce1 100644 --- a/dlls/gdiplus/region.c +++ b/dlls/gdiplus/region.c @@ -129,12 +129,32 @@ GpStatus WINGDIPAPI GdipCreateRegionPath(GpPath *path, GpRegion **region) return Ok; }
-GpStatus WINGDIPAPI GdipCreateRegionRect(GDIPCONST GpRectF *rect, GpRegion **region) +GpStatus WINGDIPAPI GdipCreateRegionRect(GDIPCONST GpRectF *rect, + GpRegion **region) { - FIXME("(%p, %p): stub\n", rect, region); + GpPath *path; + GpStatus stat;
- *region = NULL; - return NotImplemented; + if (!(rect && region)) + return InvalidParameter; + + TRACE("%p, %p\n", rect, region); + + *region = GdipAlloc(sizeof(GpRegion*)); + if(!*region) + return OutOfMemory; + + stat = GdipCreatePath(FillModeAlternate, &path); + if (stat != Ok) { GdipDeleteRegion(*region); return stat;} + stat = GdipAddPathRectangle(path, rect->X, rect->Y, + rect->Width, rect->Height); + if (stat != Ok) { GdipDeleteRegion(*region); return stat;} + stat = GdipCreateRegionPath(path, region); + if (stat != Ok) { GdipDeleteRegion(*region); return stat;} + + GdipDeletePath(path); + + return Ok; }
This doesn't look right. See the GdipGetRegionData tests (and extend them to add paths), these are supposed to help understanding how regions are stored. It looks to me that a region is stored as a sequence of rects and paths that are combined with various CombineMode ops. I'll send a patch that adds paths to the test in a bit. Huw. -- Huw Davies huw(a)codeweavers.com
On Wed, Jul 09, 2008 at 11:58:23AM +0100, Huw Davies wrote:
This doesn't look right. See the GdipGetRegionData tests (and extend them to add paths), these are supposed to help understanding how regions are stored. It looks to me that a region is stored as a sequence of rects and paths that are combined with various CombineMode ops.
Hi Adam, In fact you want to store the region elements as a binary tree, something like this: typedef enum element_type { rect_element = 0x10000000, path_element, empty_element, infinite_element } element_type_t; typedef struct region_element { DWORD type_or_op; /* One of the CombineModes or element_type_t */ union { GpRectF rect; GpPath *path; struct { struct region_element *first; struct region_element *second; } operands; } elem_data; } region_element_t; struct GpRegion { region_element_t root; }; A region element is either a rect, path or a combining op (in which case it has two children, 'first' and 'second'). The data returned by GdipGetRegionData is then basically what happens if you walk the tree, favouring the 'first' node at every branch. Does that make sense? Huw. -- Huw Davies huw(a)codeweavers.com
I was just about to reply to your first mail when I found this pop up. Anyway, To quote MSDN: "The Region class describes an area of the display surface. The area can be any shape. In other words, the boundary of the area can be a combination of curved and straight lines. Regions can also be created from the interiors of rectangles, paths, or a combination of these. Regions are used in clipping and hit-testing operations." Because the area can be any shape, including curved lines, I chose GpPath as the base object. I imagined the data you're talking about stored in an array of regionOperation structs, or something of the like - so we still have the ability to do non-rectangular hit-testing and can now getRegionData. I'll write some more tests to confirm/deny this behavior. On Wed, 2008-07-09 at 15:41 +0100, Huw Davies wrote:
On Wed, Jul 09, 2008 at 11:58:23AM +0100, Huw Davies wrote:
This doesn't look right. See the GdipGetRegionData tests (and extend them to add paths), these are supposed to help understanding how regions are stored. It looks to me that a region is stored as a sequence of rects and paths that are combined with various CombineMode ops.
Hi Adam,
In fact you want to store the region elements as a binary tree, something like this:
typedef enum element_type { rect_element = 0x10000000, path_element, empty_element, infinite_element } element_type_t;
typedef struct region_element { DWORD type_or_op; /* One of the CombineModes or element_type_t */ union { GpRectF rect; GpPath *path; struct { struct region_element *first; struct region_element *second; } operands; } elem_data; } region_element_t; I had something in mind that resembled this, but without a union. That's a better idea.
struct GpRegion { region_element_t root; };
A region element is either a rect, path or a combining op (in which case it has two children, 'first' and 'second'). The data returned by GdipGetRegionData is then basically what happens if you walk the tree, favouring the 'first' node at every branch. I had an idea of something like that.
Does that make sense? All of it.
Huw.
I didn't mean to imply that I ignored the region.c header, but I didn't use any of its information for the bounds tests yet, so I didn't add them to the struct (and I've learned to add things as needed). Thanks for the additional help.
On Wed, Jul 09, 2008 at 10:56:48AM -0400, Adam Petaccia wrote:
I didn't mean to imply that I ignored the region.c header, but I didn't use any of its information for the bounds tests yet, so I didn't add them to the struct (and I've learned to add things as needed).
The bounds checks are really the second stage. I'd get the CreateRegion functions working correctly first of all, and check that you're happy that the GetRegionData stuff could be made to work (even if you don't actually code it). Once you have that, you know you're storing the data in a similar way to native gdiplus. Then you can go on to work on the bounds (which would appear to be non-trivial - good luck!). Huw. -- Huw Davies huw(a)codeweavers.com
participants (2)
-
Adam Petaccia -
Huw Davies