[CGD2D] Migrate the text drawing code from CGContextCairo. #1729
Conversation
|
With #1727, all of the enabled drawing tests pass! |
|
Therefore, this branch will merge into the merge branch. |
| // Text Drawing | ||
| CGTextDrawingMode textDrawingMode = kCGTextFill; | ||
| woc::StrongCF<CGFontRef> font; | ||
| CGFloat fontSize; |
There was a problem hiding this comment.
perhaps this needs zero initialization.
There was a problem hiding this comment.
or the default font size value
| // Text Drawing | ||
| CGTextDrawingMode textDrawingMode = kCGTextFill; | ||
| woc::StrongCF<CGFontRef> font; | ||
| CGFloat fontSize; |
There was a problem hiding this comment.
or the default font size value
| */ | ||
| void CGContextSetTextDrawingMode(CGContextRef context, CGTextDrawingMode mode) { | ||
| NOISY_RETURN_IF_NULL(context); | ||
| UNIMPLEMENTED(); | ||
| auto& state = context->CurrentGState(); |
There was a problem hiding this comment.
nit: oneline these? (context->CurrentGState().textDrawingMode = mode)
DHowett-MSFT
force-pushed
the
201701-cgtextbro
branch
from
297a9b5 to
d073b68
Compare
| // * Stroke: fully opaque black | ||
| // If a context does not support alpha, the default fill looks like fully opaque black. | ||
| CGContextSetRGBFillColor(context, 0, 0, 0, 0); | ||
| // * All colors are fully opaque black. |
There was a problem hiding this comment.
The commit it is in is titled "Fix the default ... from the reference platform."
| UNIMPLEMENTED(); | ||
| NOISY_RETURN_IF_NULL(font); | ||
| auto& state = context->CurrentGState(); | ||
| state.font = font; |
There was a problem hiding this comment.
context->CurrentGState().font= font;
| */ | ||
| void CGContextSetTextDrawingMode(CGContextRef context, CGTextDrawingMode mode) { | ||
| NOISY_RETURN_IF_NULL(context); | ||
| UNIMPLEMENTED(); | ||
| auto& state = context->CurrentGState(); | ||
| state.textDrawingMode = mode; |
There was a problem hiding this comment.
context->CurrentGState().mode= mode;
| */ | ||
| void CGContextSelectFont(CGContextRef context, const char* name, CGFloat size, CGTextEncoding encoding) { | ||
| NOISY_RETURN_IF_NULL(context); | ||
| UNIMPLEMENTED(); | ||
|
|
||
| auto fontName = woc::MakeStrongCF<CFStringRef>(CFStringCreateWithCString(nullptr, name, kCFStringEncodingUTF8)); |
There was a problem hiding this comment.
auto [](start = 4, length = 4)
we should just give it the correct type, maybe we are over abusing auto here?
There was a problem hiding this comment.
CC explicitly allows the use of auto when the type is unwieldy and immediately expressly identified by the right side
There was a problem hiding this comment.
sure, I feel like this could lead to every single instance of woc:MakeStrong/ woc:* end up being auto.
your call.
In reply to: 96745127 [](ancestors = 96745127)
| } | ||
|
|
||
| /** | ||
| @Status Interoperable | ||
| */ | ||
| void CGContextSetFontSize(CGContextRef context, CGFloat ptSize) { | ||
| NOISY_RETURN_IF_NULL(context); | ||
| UNIMPLEMENTED(); | ||
| auto& state = context->CurrentGState(); | ||
| state.fontSize = ptSize; |
There was a problem hiding this comment.
nit: context->CurrentGState().fontSize = ptSize;
| } | ||
|
|
||
| // Internal: used by CoreText. | ||
| void CGContextDrawGlyphRun(CGContextRef context, const DWRITE_GLYPH_RUN* glyphRun) { |
There was a problem hiding this comment.
i would be happy if we also check the validity of glyphRun here. It doesn't look like DrawGlyphRunis takes care of this and can crash
There was a problem hiding this comment.
This is only called by proper consumers who are aware of the contract.
There was a problem hiding this comment.
i would suggest to add it at least in the actual function (makes sense to validate it, rather than expecting the user to give it in)
In the future someone decide to be less cautious and call it, we would end up with a crash.
| @@ -1878,7 +2035,6 @@ void CGContextShowGlyphs(CGContextRef context, const CGGlyph* glyphs, unsigned c | |||
| */ | |||
| void CGContextShowGlyphsAtPositions(CGContextRef context, const CGGlyph* glyphs, const CGPoint* Lpositions, size_t count) { | |||
| NOISY_RETURN_IF_NULL(context); | |||
| // TODO(DH) GH#1070 Merge in CGContextCairo.mm's Glyph Run code. | |||
There was a problem hiding this comment.
// TODO(DH) GH#1070 Merge in CGContextCairo.mm's Gl [](start = 3, length = 52)
is this being missed? or it's will/not arrive?
There was a problem hiding this comment.
This doesn't exist. The dream isn't real.
| // Give array of advances of zero so it will use positions correctly | ||
| std::vector<FLOAT> dwriteAdvances(count, 0); | ||
| DWRITE_GLYPH_RUN run = { fontFace.Get(), state.fontSize, count, glyphs, dwriteAdvances.data(), positions.data(), FALSE, 0 }; | ||
| THROW_IF_FAILED(context->DrawGlyphRun(&run, false)); |
There was a problem hiding this comment.
THROW_IF_FAILED [](start = 4, length = 15)
i thought we settled on fast fail?
| //****************************************************************************** | ||
|
|
||
| #import <CoreGraphics/CGGeometry.h> | ||
| #import <Starboard.h> |
DHowett-MSFT
force-pushed
the
201701-cgtextbro
branch
from
d073b68 to
92ed07c
Compare
DHowett-MSFT
dismissed
msft-Jeyaram’s stale review
I fixed the legitimate complaints you had before you submitted this status.
|
|
Fixes #1634.
This change is