Closed Bug 543892 Opened 14 years ago Closed 14 years ago

DirectWrite Cairo Backend transformation issues

Categories

(Core :: Graphics, defect)

Product:
Core
Component:
Graphics
Platform:
x86
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: bas.schouten, Assigned: bas.schouten)

References

Details

Attachments

(1 file, 1 obsolete file)

Fix for cairo DirectWrite backend transformations v2
25.82 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review 
The DirectWrite cairo backend was incorrectly dealing with transformations. And inconsistently dealing with them for D2D and GDI. I added a patch that fixes that, it properly deals with the font matrix, and cleans up some of how the matrices are treated. It also properly transforms D2D brushes on text to make sure the pattern is correctly overlaid on the text. It also fixes the mask handling in the transformed fallback path to look much better.

Also corrected inconsistent newlines in the license plate.

This patch is required to make fake italics work for DirectWrite.
Attachment #424905 - Flags: review?(jmuizelaar)
Comment on attachment 424905 [details] [diff] [review]
Fix for cairo DirectWrite backend transformations

First, it would be easier to review if these changes we're separated out into separate patches.

>diff --git a/gfx/cairo/cairo/src/cairo-dwrite-font.cpp b/gfx/cairo/cairo/src/cairo-dwrite-font.cpp
>--- a/gfx/cairo/cairo/src/cairo-dwrite-font.cpp
>+++ b/gfx/cairo/cairo/src/cairo-dwrite-font.cpp
>@@ -293,41 +300,31 @@ _cairo_dwrite_font_face_scaled_font_crea
> {
>     cairo_dwrite_font_face_t *font_face = static_cast<cairo_dwrite_font_face_t*>(abstract_face);
> 
>     // Must do malloc and not C++ new, since Cairo frees this.
>     cairo_dwrite_scaled_font_t *dwriteFont = (cairo_dwrite_scaled_font_t*)malloc(sizeof(cairo_dwrite_scaled_font_t));
>     *font = reinterpret_cast<cairo_scaled_font_t*>(dwriteFont);
>     _cairo_scaled_font_init(&dwriteFont->base, &font_face->base, font_matrix, ctm, options, &_cairo_dwrite_scaled_font_backend);
> 
>-    dwriteFont->dwritematrix.dx = (FLOAT)font_matrix->x0;
>-    dwriteFont->dwritematrix.dy = (FLOAT)font_matrix->y0;
>-    dwriteFont->dwritematrix.m11 = (FLOAT)font_matrix->xx;
>-    dwriteFont->dwritematrix.m12 = (FLOAT)font_matrix->xy;
>-    dwriteFont->dwritematrix.m21 = (FLOAT)font_matrix->yx;
>-    dwriteFont->dwritematrix.m22 = (FLOAT)font_matrix->yy;
>-
>     cairo_font_extents_t extents;
> 
>     DWRITE_FONT_METRICS metrics;
>     font_face->dwriteface->GetMetrics(&metrics);
> 
>     extents.ascent = (FLOAT)metrics.ascent / metrics.designUnitsPerEm;
>     extents.descent = (FLOAT)metrics.descent / metrics.designUnitsPerEm;
>     extents.height = (FLOAT)(metrics.ascent + metrics.descent + metrics.lineGap) / metrics.designUnitsPerEm;
>     extents.max_x_advance = 14.0;
>     extents.max_y_advance = 0.0;
>-	
>-    dwriteFont->ctm_inverse = dwriteFont->base.ctm;
>-    cairo_matrix_invert (&dwriteFont->ctm_inverse);
>-    /* Change these since we apply the transformation to an inverted
>-     * y axis.
>-     */
>-    dwriteFont->ctm_inverse.xy = - dwriteFont->ctm_inverse.xy;
>-    dwriteFont->ctm_inverse.yx = - dwriteFont->ctm_inverse.yx;
>+
>+    dwriteFont->mat = dwriteFont->base.ctm;
>+    cairo_matrix_multiply(&dwriteFont->mat, &dwriteFont->mat, font_matrix);
>+    dwriteFont->mat_inverse = dwriteFont->mat;
>+    cairo_matrix_invert (&dwriteFont->mat_inverse);
> 
>     return _cairo_scaled_font_set_metrics (*font, &extents);
> }
> 
> /* Implementation cairo_dwrite_scaled_font_backend_t */
> void
> _cairo_dwrite_scaled_font_fini(void *scaled_font)
> {
>@@ -407,69 +404,73 @@ _cairo_dwrite_scaled_show_glyphs(void			
>     } else {
> 	cairo_dwrite_scaled_font_t *dwritesf =
> 	    static_cast<cairo_dwrite_scaled_font_t*>(scaled_font);
> 	UINT16 *indices = new UINT16[num_glyphs];
> 	DWRITE_GLYPH_OFFSET *offsets = new DWRITE_GLYPH_OFFSET[num_glyphs];
> 	FLOAT *advances = new FLOAT[num_glyphs];
> 	BOOL transform = FALSE;
> 
>-	if (dwritesf->base.ctm.xy == 0 && dwritesf->base.ctm.yx == 0 &&
>-	    dwritesf->base.ctm.xx == 1.0 && dwritesf->base.ctm.yy == 1.0) {
>-	    for (int i = 0; i < num_glyphs; i++) {
>-		indices[i] = (WORD) glyphs[i].index;
>-		offsets[i].ascenderOffset = -(FLOAT)(glyphs[i].y - dest_y);
>-		offsets[i].advanceOffset = (FLOAT)(glyphs[i].x - dest_x);
>-		advances[i] = 0.0;
>-	    }
>-	} else {
>-	    transform = TRUE;
>-
>-	    for (int i = 0; i < num_glyphs; i++) {
>-		indices[i] = (WORD) glyphs[i].index;
>-		double x = glyphs[i].x;
>-		double y = -glyphs[i].y;
>-		cairo_matrix_transform_point(&dwritesf->ctm_inverse, &x, &y);
>-		// Since we will multiply by our ctm matrix later for rotation effects
>-		// and such, adjust positions by the inverse matrix now.
>-		offsets[i].ascenderOffset = (FLOAT)y;
>-		offsets[i].advanceOffset = (FLOAT)x;
>-		advances[i] = 0.0;
>-	    }
>-	}
>-	if (generic_surface->device_transform.xx != 1.0) {
>-	    int a = 1;
>-	}
> 	DWRITE_GLYPH_RUN run;
> 	run.bidiLevel = 0;
>-	run.fontEmSize = (FLOAT)dwritesf->base.font_matrix.yy;
> 	run.fontFace = ((cairo_dwrite_font_face_t*)dwritesf->base.font_face)->dwriteface;
> 	run.glyphIndices = indices;
> 	run.glyphCount = num_glyphs;
> 	run.isSideways = FALSE;
> 	run.glyphOffsets = offsets;
> 	run.glyphAdvances = advances;
>     	IDWriteGlyphRunAnalysis *analysis;
>+
>+	if (dwritesf->mat.xy == 0 && dwritesf->mat.yx == 0 &&
>+	    dwritesf->mat.xx == dwritesf->base.font_matrix.xx && 
>+	    dwritesf->mat.yy == dwritesf->base.font_matrix.yy) {
>+
>+	    for (int i = 0; i < num_glyphs; i++) {
>+		indices[i] = (WORD) glyphs[i].index;
>+		// Since we will multiply by our ctm matrix later for rotation effects
>+		// and such, adjust positions by the inverse matrix now.
>+		offsets[i].ascenderOffset = (FLOAT)dest_y - (FLOAT)glyphs[i].y;
>+		offsets[i].advanceOffset = (FLOAT)glyphs[i].x - dest_x;
>+		advances[i] = 0.0;
>+	    }
>+	    run.fontEmSize = (FLOAT)dwritesf->base.font_matrix.yy;
>+	} else {
>+	    transform = TRUE;
>+
>+	    for (int i = 0; i < num_glyphs; i++) {
>+		indices[i] = (WORD) glyphs[i].index;
>+		double x = glyphs[i].x - dest_x;
>+		double y = glyphs[i].y - dest_y;
>+		cairo_matrix_transform_point(&dwritesf->mat_inverse, &x, &y);
>+		// Since we will multiply by our ctm matrix later for rotation effects
>+		// and such, adjust positions by the inverse matrix now.
>+		offsets[i].ascenderOffset = -(FLOAT)y;
>+		offsets[i].advanceOffset = (FLOAT)x;
>+		advances[i] = 0.0;
>+	    }
>+	    run.fontEmSize = (FLOAT)1.0;

Use 1.0f

>+	}
>+
> 	if (!transform) {
> 	    DWriteFactory::Instance()->CreateGlyphRunAnalysis(&run,
> 							      1.0f,
> 							      NULL,
> 							      DWRITE_RENDERING_MODE_CLEARTYPE_NATURAL_SYMMETRIC,
> 							      DWRITE_MEASURING_MODE_NATURAL,
> 							      0,
> 							      0,
> 							      &analysis);
> 	} else {
> 	    DWRITE_MATRIX dwmatrix;
>-	    dwmatrix.dx = (FLOAT)dwritesf->base.ctm.x0;
>-	    dwmatrix.dy = (FLOAT)dwritesf->base.ctm.y0;
>-	    dwmatrix.m11 = (FLOAT)dwritesf->base.ctm.xx;
>-	    dwmatrix.m12 = (FLOAT)dwritesf->base.ctm.xy;
>-	    dwmatrix.m21 = (FLOAT)dwritesf->base.ctm.yx;
>-	    dwmatrix.m22 = (FLOAT)dwritesf->base.ctm.yy;
>+	    dwmatrix.dx = (FLOAT)dwritesf->mat.x0;
>+	    dwmatrix.dy = (FLOAT)dwritesf->mat.y0;
>+	    dwmatrix.m11 = (FLOAT)dwritesf->mat.xx;
>+	    dwmatrix.m12 = (FLOAT)dwritesf->mat.yx;
>+	    dwmatrix.m21 = (FLOAT)dwritesf->mat.xy;
>+	    dwmatrix.m22 = (FLOAT)dwritesf->mat.yy;

I think it's probably worth having a cairo_matrix to DWRITE_MATRIX function. We can hide all the
casting in there and we're less likely to get accidentally get the ordering wrong. We should also probably do the same for D2D1::Matrix3x2F.

> 	    DWriteFactory::Instance()->CreateGlyphRunAnalysis(&run,
> 							      1.0f,
> 							      &dwmatrix,
> 							      DWRITE_RENDERING_MODE_CLEARTYPE_NATURAL_SYMMETRIC,
> 							      DWRITE_MEASURING_MODE_NATURAL,
> 							      0,
> 							      0,
> 							      &analysis);
>@@ -487,20 +488,20 @@ _cairo_dwrite_scaled_show_glyphs(void			
> 
> 	cairo_image_surface_t *mask_surface = 
> 	    (cairo_image_surface_t*)cairo_image_surface_create(CAIRO_FORMAT_ARGB32, width, height);
> 
> 	cairo_surface_flush(&mask_surface->base);
> 
> 	for (unsigned int y = 0; y < height; y++) {
> 	    for (unsigned int x = 0; x < width; x++) {
>-		mask_surface->data[y * mask_surface->stride + x * 4] = surface[y * width * 3 + x * 3 + 2];
>+		mask_surface->data[y * mask_surface->stride + x * 4] = surface[y * width * 3 + x * 3 + 1];
> 		mask_surface->data[y * mask_surface->stride + x * 4 + 1] = surface[y * width * 3 + x * 3 + 1];
>-		mask_surface->data[y * mask_surface->stride + x * 4 + 2] = surface[y * width * 3 + x * 3];
>-		mask_surface->data[y * mask_surface->stride + x * 4 + 3] = surface[y * width * 3 + x * 3 + 2];
>+		mask_surface->data[y * mask_surface->stride + x * 4 + 2] = surface[y * width * 3 + x * 3 + 1];
>+		mask_surface->data[y * mask_surface->stride + x * 4 + 3] = surface[y * width * 3 + x * 3 + 1];
> 	    }
> 	}
> 	cairo_surface_mark_dirty(&mask_surface->base);
> 
> 	pixman_image_set_component_alpha(mask_surface->pixman_image, 1);
> 
> 	cairo_surface_pattern_t mask;
> 	_cairo_pattern_init_for_surface (&mask, &mask_surface->base);
>@@ -770,45 +771,45 @@ _cairo_dwrite_scaled_font_init_glyph_sur
>     if (status)
> 	goto FAIL;
> 
>     DWRITE_GLYPH_RUN run;
>     FLOAT advance = 0;
>     UINT16 index = (UINT16)glyph.index;
>     DWRITE_GLYPH_OFFSET offset;
>     double x = glyph.x;
>-    double y = -glyph.y;
>-    cairo_matrix_transform_point(&scaled_font->ctm_inverse, &x, &y);
>+    double y = glyph.y;
>+    cairo_matrix_transform_point(&scaled_font->mat_inverse, &x, &y);
>     offset.advanceOffset = (FLOAT)x;
>-    offset.ascenderOffset = (FLOAT)y;
>+    offset.ascenderOffset = -(FLOAT)y;

Can you add a comment explaining this computation/mapping of points

> 
>     RECT area;
>     area.top = 0;
>     area.bottom = height;
>     area.left = 0;
>     area.right = width;
> 
>     run.glyphCount = 1;
>     run.glyphAdvances = &advance;
>     run.fontFace = ((cairo_dwrite_font_face_t*)scaled_font->base.font_face)->dwriteface;
>-    run.fontEmSize = (FLOAT)scaled_font->base.font_matrix.yy;
>+    run.fontEmSize = (FLOAT)1.0;

use 1.0f or even just 1.0 instead

>     run.bidiLevel = 0;
>     run.glyphIndices = &index;
>     run.isSideways = FALSE;
>     run.glyphOffsets = &offset;
> 
>     DWRITE_MATRIX matrix;
>-    matrix.m11 = (FLOAT)scaled_font->base.ctm.xx;
>-    matrix.m12 = (FLOAT)scaled_font->base.ctm.yx;
>-    matrix.m21 = (FLOAT)scaled_font->base.ctm.xy;
>-    matrix.m22 = (FLOAT)scaled_font->base.ctm.yy;
>-    matrix.dx = (FLOAT)scaled_font->base.ctm.x0;
>-    matrix.dy = (FLOAT)scaled_font->base.ctm.y0;
>+    matrix.m11 = (FLOAT)scaled_font->mat.xx;
>+    matrix.m12 = (FLOAT)scaled_font->mat.yx;
>+    matrix.m21 = (FLOAT)scaled_font->mat.xy;
>+    matrix.m22 = (FLOAT)scaled_font->mat.yy;
>+    matrix.dx = (FLOAT)scaled_font->mat.x0;
>+    matrix.dy = (FLOAT)scaled_font->mat.y0;
> 
>-    status = _dwrite_draw_glyphs_to_gdi_surface_d2d (surface, &matrix, &run, RGB(0,0,0), area);
>+    status = _dwrite_draw_glyphs_to_gdi_surface_gdi (surface, &matrix, &run, RGB(0,0,0), area);
>     if (status)
> 	goto FAIL;
> 
>     GdiFlush();
> 
>     image = _compute_a8_mask (surface);
>     status = (cairo_int_status_t)image->status;
>     if (status)
>@@ -1005,17 +1006,17 @@ cairo_dwrite_show_glyphs_on_surface(void
> 
>     /* We can only handle opaque solid color sources */
>     if (!_cairo_pattern_is_opaque_solid(source))
> 	return CAIRO_INT_STATUS_UNSUPPORTED;
> 
>     /* We can only handle operator SOURCE or OVER with the destination
>      * having no alpha */
>     if ((op != CAIRO_OPERATOR_SOURCE && op != CAIRO_OPERATOR_OVER) ||
>-	(dst->format != CAIRO_FORMAT_RGB24) && (dst->format != CAIRO_FORMAT_ARGB32))
>+	(dst->format != CAIRO_FORMAT_RGB24))
> 	return CAIRO_INT_STATUS_UNSUPPORTED;
> 
> 
>     /* It is vital that dx values for dxy_buf are calculated from the delta of
>      * _logical_ x coordinates (not user x coordinates) or else the sum of all
>      * previous dx values may start to diverge from the current glyph's x
>      * coordinate due to accumulated rounding error. As a result strings could
>      * be painted shorter or longer than expected. */
>@@ -1039,20 +1040,20 @@ cairo_dwrite_show_glyphs_on_surface(void
> 	if (glyphs[i].y < smallestY) {
> 	    smallestY = (INT32)glyphs[i].y;
> 	}
> 	if (glyphs[i].y > largestY) {
> 	    largestY = (INT32)glyphs[i].y;
> 	}
>     }
>     RECT fontArea;
>-    fontArea.left = smallestX - (INT32)dwritesf->dwritematrix.m22;
>-    fontArea.right = largestX + (INT32)dwritesf->dwritematrix.m22 * 2;
>-    fontArea.top = smallestY - (INT32)dwritesf->dwritematrix.m22;
>-    fontArea.bottom = largestY + (INT32)dwritesf->dwritematrix.m22 * 2;
>+    fontArea.left = smallestX - (INT32)scaled_font->font_matrix.xx;
>+    fontArea.right = largestX + (INT32)scaled_font->font_matrix.xx * 2;
>+    fontArea.top = smallestY - (INT32)scaled_font->font_matrix.yy;
>+    fontArea.bottom = largestY + (INT32)scaled_font->font_matrix.yy * 2;

It would be better if we were more explicit about the integer conversion used here.
Do we really want truncation? I also don't understand how/why this computation works.

>     if (fontArea.left < 0)
> 	fontArea.left = 0;
>     if (fontArea.top > 0)
> 	fontArea.top = 0;
>     if (fontArea.bottom > dst->extents.height) {
> 	fontArea.bottom = dst->extents.height;
>     }
>     if (fontArea.right > dst->extents.width) {
>@@ -1063,66 +1064,67 @@ cairo_dwrite_show_glyphs_on_surface(void
> 	return CAIRO_INT_STATUS_SUCCESS;
>     }
>     if (fontArea.right > dst->extents.width) {
> 	fontArea.right = dst->extents.width;
>     }
>     if (fontArea.bottom > dst->extents.height) {
> 	fontArea.bottom = dst->extents.height;
>     }
>-    if (scaled_font->ctm.xy == 0 && scaled_font->ctm.yx == 0 &&
>-	scaled_font->ctm.xx == 1.0 && scaled_font->ctm.yy == 1.0) {
>-	cairo_matrix_t mat = scaled_font->ctm;
>-	cairo_matrix_invert (&mat);
>+
>+    DWRITE_GLYPH_RUN run;
>+    run.bidiLevel = 0;
>+    run.fontFace = dwriteff->dwriteface;
>+    run.glyphIndices = indices;
>+    run.glyphCount = num_glyphs;
>+    run.isSideways = FALSE;
>+    run.glyphOffsets = offsets;
>+    run.glyphAdvances = advances;
>+    if (dwritesf->mat.xy == 0 && dwritesf->mat.yx == 0 &&
>+	dwritesf->mat.xx == scaled_font->font_matrix.xx && 
>+	dwritesf->mat.yy == scaled_font->font_matrix.yy) {
>+
> 	for (int i = 0; i < num_glyphs; i++) {
> 	    indices[i] = (WORD) glyphs[i].index;
> 	    // Since we will multiply by our ctm matrix later for rotation effects
> 	    // and such, adjust positions by the inverse matrix now.
> 	    offsets[i].ascenderOffset = (FLOAT)(fontArea.top - glyphs[i].y);
> 	    offsets[i].advanceOffset = (FLOAT)(glyphs[i].x - fontArea.left);
> 	    advances[i] = 0.0;
> 	}
>+        run.fontEmSize = (FLOAT)scaled_font->font_matrix.yy;
>     } else {
> 	transform = TRUE;
> 
> 	for (int i = 0; i < num_glyphs; i++) {
> 	    indices[i] = (WORD) glyphs[i].index;
> 	    double x = glyphs[i].x - fontArea.left;
>-	    double y = fontArea.top - glyphs[i].y;
>-	    cairo_matrix_transform_point(&dwritesf->ctm_inverse, &x, &y);
>+	    double y = glyphs[i].y - fontArea.top;
>+	    cairo_matrix_transform_point(&dwritesf->mat_inverse, &x, &y);
> 	    // Since we will multiply by our ctm matrix later for rotation effects
> 	    // and such, adjust positions by the inverse matrix now.
>-	    offsets[i].ascenderOffset = (FLOAT)y;
>+	    offsets[i].ascenderOffset = -(FLOAT)y;

A comment about why this is negative wouldn't hurt :)

> 	    offsets[i].advanceOffset = (FLOAT)x;
> 	    advances[i] = 0.0;
> 	}
>+	run.fontEmSize = (FLOAT)1.0;

1.0f

>     }
>-
>-    DWRITE_GLYPH_RUN run;
>-    run.bidiLevel = 0;
>-    run.fontEmSize = (FLOAT)scaled_font->font_matrix.yy;
>-    run.fontFace = dwriteff->dwriteface;
>-    run.glyphIndices = indices;
>-    run.glyphCount = num_glyphs;
>-    run.isSideways = FALSE;
>-    run.glyphOffsets = offsets;
>-    run.glyphAdvances = advances;
>     
>     cairo_solid_pattern_t *solid_pattern = (cairo_solid_pattern_t *)source;
>     COLORREF color = RGB(((int)solid_pattern->color.red_short) >> 8,
> 		((int)solid_pattern->color.green_short) >> 8,
> 		((int)solid_pattern->color.blue_short) >> 8);
> 
>     DWRITE_MATRIX matrix;
>-    matrix.m11 = (FLOAT)scaled_font->ctm.xx;
>-    matrix.m12 = (FLOAT)scaled_font->ctm.yx;
>-    matrix.m21 = (FLOAT)scaled_font->ctm.xy;
>-    matrix.m22 = (FLOAT)scaled_font->ctm.yy;
>-    matrix.dx = (FLOAT)scaled_font->ctm.x0;
>-    matrix.dy = (FLOAT)scaled_font->ctm.y0;
>+    matrix.m11 = (FLOAT)dwritesf->mat.xx;
>+    matrix.m12 = (FLOAT)dwritesf->mat.yx;
>+    matrix.m21 = (FLOAT)dwritesf->mat.xy;
>+    matrix.m22 = (FLOAT)dwritesf->mat.yy;
>+    matrix.dx = (FLOAT)dwritesf->mat.x0;
>+    matrix.dy = (FLOAT)dwritesf->mat.y0;

conversion function

>@@ -1210,64 +1212,65 @@ cairo_dwrite_show_glyphs_on_d2d_surface(
>      * coordinate due to accumulated rounding error. As a result strings could
>      * be painted shorter or longer than expected. */
> 
>     UINT16 *indices = new UINT16[num_glyphs];
>     DWRITE_GLYPH_OFFSET *offsets = new DWRITE_GLYPH_OFFSET[num_glyphs];
>     FLOAT *advances = new FLOAT[num_glyphs];
>     BOOL transform = FALSE;
> 
>-    if (scaled_font->ctm.xy == 0 && scaled_font->ctm.yx == 0 &&
>-	scaled_font->ctm.xx == 1.0 && scaled_font->ctm.yy == 1.0) {
>+    DWRITE_GLYPH_RUN run;
>+    run.bidiLevel = 0;
>+    run.fontFace = dwriteff->dwriteface;
>+    run.glyphIndices = indices;
>+    run.glyphCount = num_glyphs;
>+    run.isSideways = FALSE;
>+    run.glyphOffsets = offsets;
>+    run.glyphAdvances = advances;
>+
>+    if (dwritesf->mat.xy == 0 && dwritesf->mat.yx == 0 &&
>+	dwritesf->mat.xx == scaled_font->font_matrix.xx && 
>+	dwritesf->mat.yy == scaled_font->font_matrix.yy) {
>+	// Fast route, don't actually use a transform but just
>+        // set the correct font size.
>+        run.fontEmSize = (FLOAT)scaled_font->font_matrix.yy;
>+
> 	for (int i = 0; i < num_glyphs; i++) {
> 	    indices[i] = (WORD) glyphs[i].index;
> 	    // Since we will multiply by our ctm matrix later for rotation effects
> 	    // and such, adjust positions by the inverse matrix now.
> 	    offsets[i].ascenderOffset = -(FLOAT)(glyphs[i].y);
> 	    offsets[i].advanceOffset = (FLOAT)(glyphs[i].x);
> 	    advances[i] = 0.0;
> 	}
>     } else {
> 	transform = TRUE;
>-	// D2D does not invert y axis.
>-	dwritesf->ctm_inverse.xy = - dwritesf->ctm_inverse.xy;
>-	dwritesf->ctm_inverse.yx = - dwritesf->ctm_inverse.yx;

glad to see this go :)

> 	for (int i = 0; i < num_glyphs; i++) {
> 	    indices[i] = (WORD) glyphs[i].index;
> 	    double x = glyphs[i].x;
> 	    double y = glyphs[i].y;
>-	    cairo_matrix_transform_point(&dwritesf->ctm_inverse, &x, &y);
>+	    cairo_matrix_transform_point(&dwritesf->mat_inverse, &x, &y);
> 	    // Since we will multiply by our ctm matrix later for rotation effects
> 	    // and such, adjust positions by the inverse matrix now.
> 	    offsets[i].ascenderOffset = -(FLOAT)y;
> 	    offsets[i].advanceOffset = (FLOAT)x;
> 	    advances[i] = 0.0;
> 	}
>-	dwritesf->ctm_inverse.xy = - dwritesf->ctm_inverse.xy;
>-	dwritesf->ctm_inverse.yx = - dwritesf->ctm_inverse.yx;
>+	// The font matrix takes care of the scaling if we have a transform,
>+	// emSize should be 1.
>+        run.fontEmSize = (FLOAT)1.0;
>     }

1.0f

>-    DWRITE_GLYPH_RUN run;
>-    run.bidiLevel = 0;
>-    run.fontEmSize = (FLOAT)scaled_font->font_matrix.yy;
>-    run.fontFace = dwriteff->dwriteface;
>-    run.glyphIndices = indices;
>-    run.glyphCount = num_glyphs;
>-    run.isSideways = FALSE;
>-    run.glyphOffsets = offsets;
>-    run.glyphAdvances = advances;
>-    
>-
>-    D2D1::Matrix3x2F mat((FLOAT)scaled_font->ctm.xx,
>-			 (FLOAT)scaled_font->ctm.yx,
>-			 (FLOAT)scaled_font->ctm.xy,
>-			 (FLOAT)scaled_font->ctm.yy,
>-			 (FLOAT)scaled_font->ctm.x0,
>-			 (FLOAT)scaled_font->ctm.y0);
>+    D2D1::Matrix3x2F mat((FLOAT)dwritesf->mat.xx,
>+			 (FLOAT)dwritesf->mat.yx,
>+			 (FLOAT)dwritesf->mat.xy,
>+			 (FLOAT)dwritesf->mat.yy,
>+			 (FLOAT)dwritesf->mat.x0,
>+			 (FLOAT)dwritesf->mat.y0);

Use conversion function

> 	}
>+	if (transform) {
>+	    D2D1::Matrix3x2F mat_inverse((FLOAT)dwritesf->mat_inverse.xx,
>+					 (FLOAT)dwritesf->mat_inverse.yx,
>+					 (FLOAT)dwritesf->mat_inverse.xy,
>+					 (FLOAT)dwritesf->mat_inverse.yy,
>+					 (FLOAT)dwritesf->mat_inverse.x0,
>+					 (FLOAT)dwritesf->mat_inverse.y0);
>+	    D2D1::Matrix3x2F mat_brush;
>+
>+	    // The brush matrix needs to be multiplied with the inverted matrix
>+	    // as well, to move the brush into the space of the glyphs. Before
>+	    // the render target transformation.
>+	    brush->GetTransform(&mat_brush);
>+	    mat_brush = mat_brush * mat_inverse;
>+	    brush->SetTransform(&mat_brush);
>+	}

likewise
Attachment #424905 - Flags: review?(jmuizelaar) → review-
Updated patch, sadly still in a single diff since I didn't use mercurial queues.
Attachment #424905 - Attachment is obsolete: true
Attachment #424999 - Flags: review?(jmuizelaar)
Comment on attachment 424999 [details] [diff] [review]
Fix for cairo DirectWrite backend transformations v2

Looks good
Attachment #424999 - Flags: review?(jmuizelaar) → review+
http://hg.mozilla.org/mozilla-central/rev/350784945074
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: