Closed Bug 651017 Opened 9 years ago Closed 9 years ago

DeCOM nsIDeviceContext

Categories

(Core :: Graphics, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: zwol, Assigned: zwol)

References

(Blocks 1 open bug)

Details

Attachments

(7 files)

Didn't I just say (bug 651016) that nsIDeviceContext should be totally removed?  Yes, but deCOMming it first makes it easier to chainsaw out huge chunks (only one header to update, no worries about IIDs, etc).  Besides, I have that part done and it can go in now. :)
In addition to mechanical removal of #include "nsIDeviceContext.h" from as many files as possible, this moves the printing error codes to nsIDeviceContextSpec.h, removes some stale declarations from a few places, and removes some logic in os2/nsWindow.cpp that exactly duplicated the BaseCreate method provided by xpwidgets.
Attachment #526907 - Flags: review?(roc)
We shortly won't be able to count on XPCOM objects from the gfx module being instantiated anymore, so create a dummy service and register it for the app-startup category to ensure that gfxPlatform::Init() gets called early enough.

It might not be necessary to do_GetService this dummy service in libpr0n's module constructor, but I couldn't prove it to myself.
Attachment #526908 - Flags: review?(roc)
nsThebesDeviceContext objects register themselves as memory pressure observers so they can tell their font metrics caches to compact.  Instead, have the font metrics caches do that themselves.

Also flushes out the last remnants of gThebesGFXLog and a totally pointless call to ScriptRecordDigitSubstitution on Windows.
Attachment #526909 - Flags: review?(roc)
This does four things at once - quoting the patch header:

deCOM nsIDeviceContext; 
include-minimization on nsDeviceContext.h itself;
merge GetPrintDC() into Windows/OS/2 specific callers;
merge Shutdown() with ClearCachedSystemFonts().
Most references outside gfx not fixed up.
perl -pi -e 's/\bns(?:I|Thebes)(?=DeviceContext\b/ns/g/' \
    $(grep -rEw 'ns(I|Thebes)DeviceContext' *)

perl -pi -e 's/\bnsCOMPtr(?=<nsDeviceContext>)/nsRefPtr/g' \
    $(grep -rE '\<nsCOMPtr<nsDeviceContext>' *)
Attachment #526912 - Flags: review?(roc)
Attachment #526911 - Flags: review?(roc)
Only the font cache cares about the locale language, so the device context doesn't need to hold that pointer.
Attachment #526913 - Flags: review?(roc)
Users of CreateRenderingContextInstance can just do 'new nsRenderingContext()' now.  The CreateRenderingContext(nsIDeviceContext*, nsIWidget*) overload was folded into its sole caller.
Attachment #526914 - Flags: review?(roc)
And that's it for this bug.
Comment on attachment 526909 [details] [diff] [review]
part 3: make nsThebesDeviceContext not an nsIObserver

+    nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();

add "using namespace mozilla::services;" so we don't get this cruft inline.

+    if (obs)
+        obs->AddObserver(this, "memory-pressure", PR_FALSE);
+    if (obs)
+        obs->RemoveObserver(this, "memory-pressure");
+    if (!nsCRT::strcmp(aTopic, "memory-pressure"))
+        Compact();
+    if (!mFontCache)
+        CreateFontCache();

{}
Attachment #526909 - Flags: review?(roc) → review+
Comment on attachment 526913 [details] [diff] [review]
part 6: prune unused/unimplemented methods, push some code down to nsFontCache

+    if (!aLanguage)
+        aLanguage = mLocaleLanguage;
+    if (mFontCache)
+        mFontCache->Flush();

{}
(In reply to comment #9)
> 
> +    nsCOMPtr<nsIObserverService> obs =
> mozilla::services::GetObserverService();
> 
> add "using namespace mozilla::services;" so we don't get this cruft inline.

Will do.

> {}

Inconsistent with file style.
The file itself isn't consistent, but OK.
Depends on: 651498
This appears to have broken pref handling at startup (*sigh*).

Steps (on Windows):

1. Enter about:config
2. Set 'gfx.direct2d.disabled' to true
3. Restart the browser
4. Enter http://people.mozilla.org/~jdaggett/tests/decimalfontwaterfalls-default.html in the URL bar

Result: waterfall shows continuous size increments, indicative of DirectWrite rendering.

Expected result: step-effect with several lines rendering with the same size until the next full-pixel size up, indicative of GDI rendering.

Works: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:6.0a1) Gecko/20110419 Firefox/6.0a1

Doesn't work: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:6.0a1) Gecko/20110420 Firefox/6.0a1

I traced into the prefs code and it looks like it's not picking up a value but not reporting any error.

I have a feeling we're going to get into a "all prefs should be live" argument here.  My 1-second take on that is that these base level switches dictate some fairly big switches within gfx code, ones that affect hardware/driver checks and initialization and involve significant costs in startup time.  These also affect how text is measured and rendered, so having those prefs live means figuring out how to flush *all* text measurements.  Great flexibility but at a big cost in time and effort.  I think the real question is why is it important to make load order of components like gfx and the pref system be independent ("that's the way it's always work" doesn't count as a reason).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
John - you reopened in comment 15, but I don't think you backed out, right? If not, I'm going to call this fixed again, and the remaining work can go on in bug 651498.
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
(In reply to comment #17)
> John - you reopened in comment 15, but I don't think you backed out, right? If
> not, I'm going to call this fixed again, and the remaining work can go on in
> bug 651498.

Yes, that's fine, sorry for the noise.
You need to log in before you can comment on or make changes to this bug.