[PATCH 0/2] MR10359: gdiplus: Implement software path fill for images.
Implementation via region fill is relatively slow, and can cause issues in video playback where gdiplus paths are used to render a subtitle string once per frame. They Are Billions exhibits this issue. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10359
From: Conor McCarthy <cmccarthy@codeweavers.com> Implementation via region fill is relatively slow, and can cause issues in video playback where gdiplus paths are used to render a subtitle string once per frame. They Are Billions exhibits this issue. --- dlls/gdiplus/gdiplus_private.h | 17 +++ dlls/gdiplus/graphics.c | 186 ++++++++++++++++++++++++++++++++- dlls/gdiplus/region.c | 17 +-- 3 files changed, 200 insertions(+), 20 deletions(-) diff --git a/dlls/gdiplus/gdiplus_private.h b/dlls/gdiplus/gdiplus_private.h index 9d0a166fdeb..1e04c00313f 100644 --- a/dlls/gdiplus/gdiplus_private.h +++ b/dlls/gdiplus/gdiplus_private.h @@ -52,6 +52,21 @@ #define PIXELFORMATBPP(x) ((x) ? ((x) >> 8) & 255 : 24) +struct edge +{ + /* Represents an intersection of a path segment with a scanline */ + int x; + int y; + BOOL rising; +}; + +struct edge_list +{ + struct edge *edges; + size_t capacity; + size_t length; +}; + COLORREF ARGB2COLORREF(ARGB color); HBITMAP ARGB2BMP(ARGB color); extern INT arc2polybezier(GpPointF * points, REAL x1, REAL y1, REAL x2, REAL y2, @@ -149,6 +164,8 @@ extern GpStatus trace_path(GpGraphics *graphics, GpPath *path); typedef struct region_element region_element; extern void delete_element(region_element *element); +extern GpStatus flat_path_to_edge_list(GpPath *path, const RECT *bounds, struct edge_list *edges); + extern GpStatus get_region_hrgn(struct region_element *element, const RECT *bounds, HRGN *hrgn); extern GpStatus get_hatch_data(GpHatchStyle hatchstyle, const unsigned char **result); diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 2419eea94db..453574b7e23 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -4544,25 +4544,203 @@ end: return retval; } +static void get_flat_path_bounds(GpPath *path, RECT *bound_rect) +{ + REAL min_x, min_y, max_x, max_y; + DWORD i; + + if (!path->pathdata.Count) + return; + + min_x = max_x = path->pathdata.Points[0].X; + min_y = max_y = path->pathdata.Points[0].Y; + for (i = 1; i < path->pathdata.Count; i++) + { + min_x = min(min_x, path->pathdata.Points[i].X); + max_x = max(max_x, path->pathdata.Points[i].X); + min_y = min(min_y, path->pathdata.Points[i].Y); + max_y = max(max_y, path->pathdata.Points[i].Y); + } + + bound_rect->left = floorf(min_x); + bound_rect->top = floorf(min_y); + bound_rect->right = ceilf(max_x); + bound_rect->bottom = ceilf(max_y); +} + +static void bitmap_line_fill(GpBitmap *dst_bitmap, const DWORD *src_row, int row_x, + int start_x, int end_x, int y, CompositingMode comp_mode, int premult) +{ + int x; + + for (x = start_x; x < end_x; x++) + { + ARGB dst_color, src_color; + + src_color = src_row[x - row_x]; + + if (comp_mode == CompositingModeSourceCopy) + { + if (!(src_color & 0xff000000)) + GdipBitmapSetPixel(dst_bitmap, x, y, 0); + else + GdipBitmapSetPixel(dst_bitmap, x, y, src_color); + } + else + { + if (!(src_color & 0xff000000)) + continue; + + GdipBitmapGetPixel(dst_bitmap, x, y, &dst_color); + if (premult) + GdipBitmapSetPixel(dst_bitmap, x, y, color_over_fgpremult(dst_color, src_color)); + else + GdipBitmapSetPixel(dst_bitmap, x, y, color_over(dst_color, src_color)); + } + } +} + +static GpStatus flat_path_fill_alternate(GpGraphics *graphics, struct edge_list *edges, + const DWORD *pixel_data, const GpRect *gp_bound_rect) +{ + GpBitmap *dst_bitmap = (GpBitmap*)graphics->image; + CompositingMode comp_mode = graphics->compmode; + int premult = dst_bitmap->format & PixelFormatPAlpha; + size_t i; + + for (i = 0; i + 1 < edges->length; i += 2) + { + struct edge *edge = &edges->edges[i]; + const DWORD *row; + + assert(edge[0].y == edge[1].y); + + row = pixel_data + (edge[0].y - gp_bound_rect->Y) * gp_bound_rect->Width; + bitmap_line_fill(dst_bitmap, row, gp_bound_rect->X, edge[0].x, edge[1].x, edge[0].y, comp_mode, premult); + } + + return Ok; +} + +static GpStatus flat_path_fill_winding(GpGraphics *graphics, struct edge_list *edges, + const DWORD *pixel_data, const GpRect *gp_bound_rect) +{ + GpBitmap *dst_bitmap = (GpBitmap*)graphics->image; + CompositingMode comp_mode = graphics->compmode; + int premult = dst_bitmap->format & PixelFormatPAlpha; + int start_x, winding_count = 0; + size_t i; + + start_x = gp_bound_rect->X; + + for (i = 0; i < edges->length; i++) + { + struct edge *edge = &edges->edges[i]; + const DWORD *row; + + if (winding_count) + { + row = pixel_data + (edge->y - gp_bound_rect->Y) * gp_bound_rect->Width; + bitmap_line_fill(dst_bitmap, row, gp_bound_rect->X, start_x, edge->x, edge->y, comp_mode, premult); + } + + start_x = edge->x; + winding_count += edge->rising ? 1 : -1; + } + + return Ok; +} + +static int cmp_edges(const void *a, const void *b) +{ + const struct edge *edge1 = (struct edge*)a; + const struct edge *edge2 = (struct edge*)b; + + if (edge1->y != edge2->y) + return (edge1->y > edge2->y) - (edge1->y < edge2->y); + + return (edge1->x > edge2->x) - (edge1->x < edge2->x); +} + static GpStatus SOFTWARE_GdipFillPath(GpGraphics *graphics, GpBrush *brush, GpPath *path) { GpStatus stat; GpRegion *rgn; + GpPath *flat_path = NULL; + RECT bound_rect = {0}; + struct edge_list edge_list = { 0 }; + GpRect gp_bound_rect; + DWORD *pixel_data; if (!brush_can_fill_pixels(brush)) return NotImplemented; - /* FIXME: This could probably be done more efficiently without regions. */ + if (!graphics->image) + { + /* FIXME: This could probably be done more efficiently without regions. */ + + stat = GdipCreateRegionPath(path, &rgn); + + if (stat == Ok) + { + stat = GdipFillRegion(graphics, brush, rgn); + + GdipDeleteRegion(rgn); + } - stat = GdipCreateRegionPath(path, &rgn); + return stat; + } + + stat = GdipClonePath(path, &flat_path); if (stat == Ok) { - stat = GdipFillRegion(graphics, brush, rgn); + GpMatrix transform; - GdipDeleteRegion(rgn); + stat = get_graphics_transform(graphics, WineCoordinateSpaceGdiDevice, + CoordinateSpaceWorld, &transform); + + if (stat == Ok) + stat = GdipFlattenPath(flat_path, &transform, FlatnessDefault); + } + + if (stat == Ok) + { + get_flat_path_bounds(flat_path, &bound_rect); + stat = flat_path_to_edge_list(flat_path, &bound_rect, &edge_list); + } + + if (stat == Ok) + { + gp_bound_rect.X = bound_rect.left; + gp_bound_rect.Y = bound_rect.top; + gp_bound_rect.Width = bound_rect.right - bound_rect.left; + gp_bound_rect.Height = bound_rect.bottom - bound_rect.top; + + pixel_data = calloc(gp_bound_rect.Width * gp_bound_rect.Height, sizeof(*pixel_data)); + if (!pixel_data) + stat = OutOfMemory; + + if (stat == Ok) + stat = brush_fill_pixels(graphics, brush, pixel_data, &gp_bound_rect, gp_bound_rect.Width); + + if (stat == Ok) + { + qsort(edge_list.edges, edge_list.length, sizeof(edge_list.edges[0]), cmp_edges); + + if (path->fill == FillModeWinding) + flat_path_fill_winding(graphics, &edge_list, pixel_data, &gp_bound_rect); + else + flat_path_fill_alternate(graphics, &edge_list, pixel_data, &gp_bound_rect); + } + + free(pixel_data); + free(edge_list.edges); } + if (flat_path) + GdipDeletePath(flat_path); + return stat; } diff --git a/dlls/gdiplus/region.c b/dlls/gdiplus/region.c index c5b93e1ffa3..fd7b2a19b92 100644 --- a/dlls/gdiplus/region.c +++ b/dlls/gdiplus/region.c @@ -104,21 +104,6 @@ typedef struct packed_point short Y; } packed_point; -struct edge -{ - /* Represents an intersection of a path segment with a scanline */ - int x; - int y; - BOOL rising; -}; - -struct edge_list -{ - struct edge *edges; - size_t capacity; - size_t length; -}; - static void get_region_bounding_box(struct region_element *element, REAL *min_x, REAL *min_y, REAL *max_x, REAL *max_y, BOOL *empty, BOOL *infinite); @@ -1089,7 +1074,7 @@ static GpStatus line_to_edge_list(GpPointF p1, GpPointF p2, const RECT *bounds, return stat; } -static GpStatus flat_path_to_edge_list(GpPath *path, const RECT *bounds, struct edge_list *edges) +GpStatus flat_path_to_edge_list(GpPath *path, const RECT *bounds, struct edge_list *edges) { GpStatus stat=Ok; int i, subpath_start=0; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10359
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/gdiplus/matrix.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/dlls/gdiplus/matrix.c b/dlls/gdiplus/matrix.c index dace462eb1c..6bbf03000ed 100644 --- a/dlls/gdiplus/matrix.c +++ b/dlls/gdiplus/matrix.c @@ -365,6 +365,12 @@ GpStatus WINGDIPAPI GdipShearMatrix(GpMatrix *matrix, REAL shearX, REAL shearY, GpStatus WINGDIPAPI GdipTransformMatrixPoints(GpMatrix *matrix, GpPointF *pts, INT count) { + static const GpMatrix identity = + { + { 1.0, 0.0, + 0.0, 1.0, + 0.0, 0.0 } + }; REAL x, y; INT i; @@ -373,6 +379,9 @@ GpStatus WINGDIPAPI GdipTransformMatrixPoints(GpMatrix *matrix, GpPointF *pts, if(!matrix || !pts || count <= 0) return InvalidParameter; + if (!memcmp(matrix, &identity, sizeof(*matrix))) + return Ok; + for(i = 0; i < count; i++) { x = pts[i].X; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10359
I think we'll need to approach this differently. The Graphics object can have a clipping region, and we need to intersect the path with that region (this might be part of why the tests are failing). So we end up having to fill a region regardless. Possibly we should also intersect it with the bitmap bounds so we don't bother rendering areas outside of there. Given that, it probably makes more sense to do this in GdipFillRegion. I think this means we'll need to be able to do region combine operations on an edge list. Maybe an internal "rasterized region" structure could make sense for this? Maybe something like this: * type: empty, rect, alternate path, or winding path * bounding rectangle * edge_list (sorted) When I was planning to rework the region code, I thought I'd end up with something like that, but since the end result was going to be an HRGN, or a single pixel for the hit-testing functions, it didn't seem to make sense yet. It could also make sense to cache the rasterized version of a graphics object's clipping region + bounding rectangle. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10359#note_132663
Oops, yes, I missed the clipping region. I see two failures in the metadata test module which disappear if the old code path is taken when `clip` is not infinite. There are no other failures locally in any of the gdiplus test modules. Do you see failures elsewhere? Would it make more sense to convert all region elements to a list of horizontal spans containing only `x0, x1`, before applying `CombineMode`? Spans are simple to combine, the end result is simple to render, and we avoid complications trying to combine edge lists with different fill modes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10359#note_132719
I haven't tested it locally. I have a visual test program I'm likely to run for rendering changes like this. Converting to spans probably makes sense. In theory, we should end up with a lot of trivial operations (such as intersecting a rect with an edge-list that we already know has bounds within that rect) where we could skip it, but I'm not sure if being lazy about it is worth the complexity. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10359#note_132833
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Esme Povirk (＠madewokherd)