Closed
Bug 1509358
Opened 6 years ago
Closed 6 years ago
Replace DCFromDrawTarget with DCForMetrics
Categories
(Core :: Graphics: Text, enhancement, P3)
Core
Graphics: Text
Tracking
()
RESOLVED
FIXED
mozilla65
Tracking | Status | |
---|---|---|
firefox65 | --- | fixed |
People
(Reporter: jrmuizel, Assigned: jrmuizel)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
The GDI font code path is very rarely used, further when it used we don't ever seem to get any DrawTarget that's not Skia and so never try to pull the DC out of the DrawTarget anyways. I tested this by forcing on GDI fonts, running the browser and printing, with and without e10s.
Making this change will let us rip out a bunch of code that threads the DrawTarget through the text code.
Assignee | ||
Comment 1•6 years ago
|
||
The GDI font code path is very rarely used, further when it used we
don't ever seem to get any DrawTarget that's not Skia and so never try
to pull the DC out of the DrawTarget anyways. I tested this by forcing
on GDI fonts, running the browser and printing, with and without e10s.
Making this change will let us rip out a bunch of code that threads the
DrawTarget through the text code.
Updated•6 years ago
|
Assignee: nobody → jmuizelaar
Status: NEW → ASSIGNED
Priority: -- → P3
Comment 2•6 years ago
|
||
(In reply to Jeff Muizelaar [:jrmuizel] from comment #1)
> Created attachment 9027025 [details]
> Bug 1509358. Replace DCFromDrawTarget with DCForMetrics.
>
> The GDI font code path is very rarely used, further when it used we
> don't ever seem to get any DrawTarget that's not Skia and so never try
> to pull the DC out of the DrawTarget anyways. I tested this by forcing
> on GDI fonts, running the browser and printing, with and without e10s.
I'm curious about this -- I tried something similar, forcing the use of GDI fonts (by commenting-out the DirectWrite font-list initialization in gfxWindowsPlatform::CreatePlatformFontList), and then added an assertion in DCFromDrawTarget where it gets the DC from a win32 surface. When I then "print" (using the print-to-PDF driver), I *do* hit that assertion.
It's unclear to me how important this is; if I neuter DCFromDrawTarget such that it just goes with the whole-screen DC in all cases, the resulting PDF looks fine AFAICS. But possibly if I were printing to a raster printer driver, or something like that, results might be different?
Assignee | ||
Comment 3•6 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #2)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #1)
> > Created attachment 9027025 [details]
> > Bug 1509358. Replace DCFromDrawTarget with DCForMetrics.
> >
> > The GDI font code path is very rarely used, further when it used we
> > don't ever seem to get any DrawTarget that's not Skia and so never try
> > to pull the DC out of the DrawTarget anyways. I tested this by forcing
> > on GDI fonts, running the browser and printing, with and without e10s.
>
> I'm curious about this -- I tried something similar, forcing the use of GDI
> fonts (by commenting-out the DirectWrite font-list initialization in
> gfxWindowsPlatform::CreatePlatformFontList), and then added an assertion in
> DCFromDrawTarget where it gets the DC from a win32 surface. When I then
> "print" (using the print-to-PDF driver), I *do* hit that assertion.
Very weird. I tried this exact same thing and wasn't able to get it to trigger.
> It's unclear to me how important this is; if I neuter DCFromDrawTarget such
> that it just goes with the whole-screen DC in all cases, the resulting PDF
> looks fine AFAICS. But possibly if I were printing to a raster printer
> driver, or something like that, results might be different?
This is conceivable. However, since we hardly support GDI fonts and want to remove support for them I think it's worth taking the risk here.
Pushed by jmuizelaar@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ef8fb4300e67
Replace DCFromDrawTarget with DCForMetrics. r=jfkthame
Comment 5•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in
before you can comment on or make changes to this bug.
Description
•