fix: Correct Buffer size for #2530 #2636
Conversation
There was a problem hiding this comment.
A few comments about the changes.
I think currently we are losing some features in the change to SharpFont, isn't it?
Specifically I see much more logic in the previous GetFontFace() methods that took the font styles into account.
Sorry if I'm misunderstanding the change.
| using FreeImageAPI; | ||
| using SharpDX.Mathematics.Interop; | ||
| using SharpFont; | ||
| using Stride.Core; | ||
| using Factory = SharpDX.DirectWrite.Factory; | ||
|
|
||
| // This code was originally taken from DirectXTk but rewritten with DirectWrite |
There was a problem hiding this comment.
This comment is no longer correct, is it? It does not use DirectWrite.
| @@ -128,7 +121,7 @@ private unsafe Glyph ImportGlyph(Face face, char character, float fontSize, Font | |||
| face.LoadGlyph(index, LoadFlags.Render, LoadTarget.Normal); | |||
|
|
|||
| var glyphBitmap = face.Glyph.Bitmap; | |||
| var bufferData = new Span<byte>((byte*)glyphBitmap.Buffer, glyphBitmap.Rows * glyphBitmap.Pitch); | |||
| var bufferData = new Span<byte>((byte*)glyphBitmap.Buffer, glyphBitmap.Rows * glyphBitmap.Width); | |||
There was a problem hiding this comment.
It makes more sense that the size is Rows times Pitch. Why that results in an incorrect buffer size? Worst case should be that Width == Pitch if byte-sized and no paddings, no?
As GetFont() is a new method, if Pitch is not correctly returning the pitch, then maybe we should fix it there.
Also related: Why create a new GetFont() method returning a Face instead of modifying the already existing GetFontFace() that returns a FontFace?
I ask because maybe we should encapsulate Face inside FontFace (as it already has a correct Pitch property), and not change this line.
PR Details
Fixes an exception for some sizes
Types of changes
Checklist