On Mon Sep 4 10:14:24 2023 +0000, Nikolay Sivov wrote:
As I recall the idea was that bumping fontEmSize is more desirable than scaling outlines up, using smaller size. I think this will need some more testing for CreateGlyphRunAnalysis() itself. I haven't tried, but differences might be easier to spot in aliased mode.
Thanks for reviewing this patch! I have added a test to check the bounding box returned from `rendertarget_DrawGlyphRun`:
```c hr = IDWriteBitmapRenderTarget_SetPixelsPerDip(target, 1.0); hr = IDWriteBitmapRenderTarget_DrawGlyphRun(target, 30.0f, 65.0f, DWRITE_MEASURING_MODE_GDI_NATURAL, &run, params, RGB(255, 0, 0), &box); /* ... */ hr = IDWriteBitmapRenderTarget_SetPixelsPerDip(target, 2.0); hr = IDWriteBitmapRenderTarget_DrawGlyphRun(target, 30.0f, 65.0f, DWRITE_MEASURING_MODE_GDI_NATURAL, &run, params, RGB(255, 0, 0), &box2); /* ... */ /* bounds should be scaled */ ok(abs(box2.left - box.left * 2) <= 2, "unexpect left bounds: %ld -> %ld\n", box.left, box2.left); ok(abs(box2.right - box.right * 2) <= 2, "unexpect right bounds: %ld -> %ld\n", box.right, box2.right); ok(abs(box2.top - box.top * 2) <= 2, "unexpect top bounds: %ld -> %ld\n", box.top, box2.top); ok(abs(box2.bottom - box.bottom * 2) <= 2, "unexpect bottom bounds: %ld -> %ld\n", box.bottom, box2.bottom); ```
this check passes in Windows 10, but it fails if we just only bumping `fontEmSize` in Wine:
```plaintext font.c:1623: Test failed: unexpect left bounds: 30 -> 30 font.c:1624: Test failed: unexpect right bounds: 96 -> 156 font.c:1625: Test failed: unexpect top bounds: 40 -> 16 font.c:1626: Test failed: unexpect bottom bounds: 71 -> 77 ```
So just bumping fontEmSize is not enouth. we also need to scale `glyphAdvance`, `glyphOffsets`, as well as `originX` and `originY` manually, this is another version of this patch which can work properly:
```patch diff --git a/dlls/dwrite/gdiinterop.c b/dlls/dwrite/gdiinterop.c index 2eafe4064d2..f2361f3c701 100644 --- a/dlls/dwrite/gdiinterop.c +++ b/dlls/dwrite/gdiinterop.c @@ -347,7 +347,9 @@ static HRESULT WINAPI rendertarget_DrawGlyphRun(IDWriteBitmapRenderTarget1 *ifac DWRITE_RENDERING_MODE1 rendermode; DWRITE_GRID_FIT_MODE gridfitmode; DWRITE_TEXTURE_TYPE texturetype; + DWRITE_GLYPH_OFFSET *scaled_off; DWRITE_GLYPH_RUN scaled_run; + FLOAT *scaled_advances; IDWriteFontFace3 *fontface; RECT target_rect, bounds; HRESULT hr; @@ -429,10 +431,38 @@ static HRESULT WINAPI rendertarget_DrawGlyphRun(IDWriteBitmapRenderTarget1 *ifac return hr; }
scaled_run = *run; scaled_run.fontEmSize *= target->ppdip; + scaled_advances = NULL; + scaled_off = NULL; + if (run->glyphAdvances) + { + scaled_advances = calloc(1, sizeof(FLOAT) * run->glyphCount); + + if (!scaled_advances) + return E_OUTOFMEMORY; + for (int i = 0; i < run->glyphCount; i++) + scaled_advances[i] = run->glyphAdvances[i] * target->ppdip; + scaled_run.glyphAdvances = scaled_advances; + } + if (run->glyphOffsets) + { + scaled_off = calloc(1, sizeof(DWRITE_GLYPH_OFFSET) * run->glyphCount); + + if (!scaled_off) + return E_OUTOFMEMORY; + for (int i = 0; i < run->glyphCount; i++) + { + scaled_off[i].advanceOffset = run->glyphOffsets[i].advanceOffset * target->ppdip; + scaled_off[i].ascenderOffset = run->glyphOffsets[i].ascenderOffset * target->ppdip; + } + scaled_run.glyphOffsets = scaled_off; + } + hr = IDWriteFactory7_CreateGlyphRunAnalysis(target->factory, &scaled_run, &target->m, rendermode, measuring_mode, - gridfitmode, target->antialiasmode, originX, originY, &analysis); + gridfitmode, target->antialiasmode, originX * target->ppdip, originY * target->ppdip, &analysis); + if (scaled_advances) free((void*) scaled_advances); + if (scaled_off) free((void*) scaled_off); if (FAILED(hr)) { WARN("failed to create analysis instance, 0x%08lx\n", hr); ```
This patch will pass the check, and the sample application can render correctly. But we have to dump all `glyphAdvances` and `glyphOffsets` in this patch, I'm not sure whether it's desirable to use this version.