356899 - fix glitz on win32

Status: RESOLVED WONTFIX

(Core :: Graphics, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Make OpenGL/glitz rendering work on Win32.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: glitz on win32

This patch doesn't really change much if we're building without --enable-glitz.

With it, it makes things mostly work, albeit slowly -- I've done zero profiling or anything, but I think we're getting hurt by text rendering because it's going through like 20 different abstractions on the way to the screen.

This is really a work in progress, I just want this out of my tree :)

Comment 2

[Stuart Parmenter]

Comment on attachment 242456 [details] [diff] [review]
glitz on win32

>+    if (0) {
>+        width = (int)metrics.gmBlackBoxX;
>+        height = (int)metrics.gmBlackBoxY;
>+        baseline = height;
>+    } else {
>+        width = metrics.gmCellIncX;
>+        height = (int)(scaled_font->base.extents.ascent + scaled_font->base.extents.descent);
>+        baseline = (int)scaled_font->base.extents.ascent;
>+    }

uh?


>Index: gfx/thebes/public/gfxMatrix.h
>===================================================================
>RCS file: /cvsroot/mozilla/gfx/thebes/public/gfxMatrix.h,v
> ....
maybe check this in seperately (r=me) since it has nothing to do with this patch




>Index: gfx/thebes/src/gfxWindowsFonts.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp,v
>retrieving revision 1.62
>diff -u -8 -p -r1.62 gfxWindowsFonts.cpp
>--- gfx/thebes/src/gfxWindowsFonts.cpp	14 Oct 2006 07:25:55 -0000	1.62
>+++ gfx/thebes/src/gfxWindowsFonts.cpp	17 Oct 2006 00:44:56 -0000
>@@ -60,23 +60,37 @@
> 
> #include <math.h>
> 
> #include "prlog.h"
> static PRLogModuleInfo *gFontLog = PR_NewLogModule("winfonts");
> 
> #define ROUND(x) floor((x) + 0.5)
> 
>-inline HDC GetDCFromSurface(gfxASurface *aSurface) {
>-    if (aSurface->GetType() != gfxASurface::SurfaceTypeWin32) {
>-        NS_ERROR("gfxWindowsTextRun::MeasureOrDrawFast: Context target is not win32!");
>-        return nsnull;
>-    }
>-    return NS_STATIC_CAST(gfxWindowsSurface*, aSurface)->GetDC();
>-}
>+inline HDC GetDCFromContext(gfxContext *aContext) {
>+    nsRefPtr<gfxASurface> aSurface = aContext->CurrentSurface();
>+    gfxASurface* surf = aSurface.get();
>+
>+    switch (surf->GetType()) {
>+        case gfxASurface::SurfaceTypeWin32:
>+            return NS_STATIC_CAST(gfxWindowsSurface*, surf)->GetDC();
>+
>+#ifdef MOZ_ENABLE_GLITZ
>+        case gfxASurface::SurfaceTypeGlitz: {
>+            surf = aContext->OriginalSurface();
>+            return NS_STATIC_CAST(gfxGlitzSurface*, surf)->TargetDC();
>+        }
>+#endif
>+
>+        default:
>+            NS_ERROR("gfxWindowsTextRun::MeasureOrDrawFast: Context target is not win32!");
>+            return nsnull;
>+     }
>+ }
>+
> 
> /**********************************************************************
>  *
>  * class gfxWindowsFont
>  *
>  **********************************************************************/
> 
> gfxWindowsFont::gfxWindowsFont(const nsAString& aName, const gfxFontStyle *aFontStyle)
>@@ -544,17 +558,17 @@ gfxWindowsTextRun::MeasureOrDrawReallyFa
>     } else {
>         aWString = mString.BeginReading();
>         aLength = mString.Length();
>         if (ScriptIsComplex(aWString, aLength, SIC_COMPLEX) == S_OK)
>             return -1; // try uniscribe instead
>     }
> 
>     nsRefPtr<gfxASurface> surf = aContext->CurrentSurface();
>-    HDC aDC = GetDCFromSurface(surf);
>+    HDC aDC = GetDCFromContext(aContext);


What surface should we be calling MarkDirty() on?  The CurrentSurface() or the one used in GetDCromContext?  Should CurrentSurface() the glitz stuff in it?  I'm not sure I like this extra voodoo in here for this...




the rest of it looks OK

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #2)
> (From update of attachment 242456 [details] [diff] [review] [edit])
> >+    if (0) {
> >+        width = (int)metrics.gmBlackBoxX;
> >+        height = (int)metrics.gmBlackBoxY;
> >+        baseline = height;
> >+    } else {
> >+        width = metrics.gmCellIncX;
> >+        height = (int)(scaled_font->base.extents.ascent + scaled_font->base.extents.descent);
> >+        baseline = (int)scaled_font->base.extents.ascent;
> >+    }
> 
> uh?

I need to put a comment in there; but basically, the blackbox numbers aren't very useful, because I have no information about where relative to the baseline (or relative to the glyph point) the blackbox area should be rendered.  So instead, this creates the maximum box for each glyph.  Not ideal, but it works.


> >Index: gfx/thebes/public/gfxMatrix.h
> >===================================================================
> >RCS file: /cvsroot/mozilla/gfx/thebes/public/gfxMatrix.h,v
> > ....
> maybe check this in seperately (r=me) since it has nothing to do with this
> patch

Sure, I can do that.

> >Index: gfx/thebes/src/gfxWindowsFonts.cpp
> >===================================================================
> >RCS file: /cvsroot/mozilla/gfx/thebes/src/gfxWindowsFonts.cpp,v
> >retrieving revision 1.62
> >diff -u -8 -p -r1.62 gfxWindowsFonts.cpp
> >--- gfx/thebes/src/gfxWindowsFonts.cpp	14 Oct 2006 07:25:55 -0000	1.62
> >+++ gfx/thebes/src/gfxWindowsFonts.cpp	17 Oct 2006 00:44:56 -0000
> >@@ -60,23 +60,37 @@
> > 
> > #include <math.h>
> > 
> > #include "prlog.h"
> > static PRLogModuleInfo *gFontLog = PR_NewLogModule("winfonts");
> > 
> > #define ROUND(x) floor((x) + 0.5)
> > 
> >-inline HDC GetDCFromSurface(gfxASurface *aSurface) {
> >-    if (aSurface->GetType() != gfxASurface::SurfaceTypeWin32) {
> >-        NS_ERROR("gfxWindowsTextRun::MeasureOrDrawFast: Context target is not win32!");
> >-        return nsnull;
> >-    }
> >-    return NS_STATIC_CAST(gfxWindowsSurface*, aSurface)->GetDC();
> >-}
> >+inline HDC GetDCFromContext(gfxContext *aContext) {
> >+    nsRefPtr<gfxASurface> aSurface = aContext->CurrentSurface();
> >+    gfxASurface* surf = aSurface.get();
> >+
> >+    switch (surf->GetType()) {
> >+        case gfxASurface::SurfaceTypeWin32:
> >+            return NS_STATIC_CAST(gfxWindowsSurface*, surf)->GetDC();
> >+
> >+#ifdef MOZ_ENABLE_GLITZ
> >+        case gfxASurface::SurfaceTypeGlitz: {
> >+            surf = aContext->OriginalSurface();
> >+            return NS_STATIC_CAST(gfxGlitzSurface*, surf)->TargetDC();
> >+        }
> >+#endif
> >+
> >+        default:
> >+            NS_ERROR("gfxWindowsTextRun::MeasureOrDrawFast: Context target is not win32!");
> >+            return nsnull;
> >+     }
> >+ }
> >+
> > 
> > /**********************************************************************
> >  *
> >  * class gfxWindowsFont
> >  *
> >  **********************************************************************/
> > 
> > gfxWindowsFont::gfxWindowsFont(const nsAString& aName, const gfxFontStyle *aFontStyle)
> >@@ -544,17 +558,17 @@ gfxWindowsTextRun::MeasureOrDrawReallyFa
> >     } else {
> >         aWString = mString.BeginReading();
> >         aLength = mString.Length();
> >         if (ScriptIsComplex(aWString, aLength, SIC_COMPLEX) == S_OK)
> >             return -1; // try uniscribe instead
> >     }
> > 
> >     nsRefPtr<gfxASurface> surf = aContext->CurrentSurface();
> >-    HDC aDC = GetDCFromSurface(surf);
> >+    HDC aDC = GetDCFromContext(aContext);
> 
> 
> What surface should we be calling MarkDirty() on?  The CurrentSurface() or the
> one used in GetDCromContext?  Should CurrentSurface() the glitz stuff in it? 
> I'm not sure I like this extra voodoo in here for this...

The CurrentSurface().  MarkDirty is only needed for win32 surfaces, in which case GetDCFromContext will return the DC of the "surf" surface (which is CurrentSurface()).  For glitz surfaces, it doesn't do anything.

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

glitz is dead