Closed Bug 982275 Opened 6 years ago Closed 6 years ago

Add the ability to draw text inside layers

Categories

(Core :: Graphics: Layers, defect)

x86_64
Windows 8.1
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: bas.schouten, Assigned: bas.schouten)

Details

Attachments

(2 files, 1 obsolete file)

We should have a way of drawing text in the compositor so we can present overlaid, real-time debug information.
I have a patch with a font for this.
Attachment #8389409 - Flags: review?(jmuizelaar)
Attachment #8389409 - Attachment is obsolete: true
Attachment #8389409 - Flags: review?(jmuizelaar)
Attachment #8389421 - Flags: review?(jmuizelaar)
Nice. Don't wait for me! This looks better than my work.
Can you add the script that generated the png? And once thats loaded why don't you use a VBO and upload a geometry and the full texture atlas of characters. I guess it doesn't matter just surprised to see it this way.
Comment on attachment 8389421 [details] [diff] [review]
Allow rendering texts on a Compositor v2

Review of attachment 8389421 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/composite/TextRenderer.cpp
@@ +144,5 @@
> +  }
> +
> +  mGlyphBitmaps = Factory::CreateDataSourceSurface(IntSize(sTextureWidth, sTextureHeight), sTextureFormat);
> +
> +  mGlyphBitmaps->Map(DataSourceSurface::MapType::READ_WRITE, &mMap);

Why not just malloc some memory instead of using a DataSourceSurface?

@@ +151,5 @@
> +  png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING, nullptr, nullptr, nullptr);
> +
> +  nsCString decoded;
> +  nsDependentCString string((char*)sFontPNG, sizeof(sFontPNG) - 1);
> +  Base64Decode(string, decoded);

Why store this as a Base64 string instead of a byte array? It seems like were better off optimizing for binary size than source code size.
Attachment #8389421 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8389424 [details] [diff] [review]
Part 2: Add pref to draw layer info onto layers.

Review of attachment 8389424 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/composite/ContainerLayerComposite.cpp
@@ +137,5 @@
> +  aLayer->PrintInfo(layerInfo, "");
> +
> +  nsIntRegion visibleRegion = aLayer->GetVisibleRegion();
> +
> +  uint32_t maxWidth = visibleRegion.GetBounds().width < 500 ? visibleRegion.GetBounds().width : 500;

Maybe use min()?
Attachment #8389424 - Flags: review?(jmuizelaar) → review+
(In reply to Jeff Muizelaar [:jrmuizel] from comment #8)
> Comment on attachment 8389424 [details] [diff] [review]
> Part 2: Add pref to draw layer info onto layers.
> 
> Review of attachment 8389424 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/composite/ContainerLayerComposite.cpp
> @@ +137,5 @@
> > +  aLayer->PrintInfo(layerInfo, "");
> > +
> > +  nsIntRegion visibleRegion = aLayer->GetVisibleRegion();
> > +
> > +  uint32_t maxWidth = visibleRegion.GetBounds().width < 500 ? visibleRegion.GetBounds().width : 500;
> 
> Maybe use min()?

I totally missed this comment, sorry :(. I'll push it in a follow-up soon.
(In reply to Andreas Gal :gal from comment #6)
> Can you add the script that generated the png? And once thats loaded why
> don't you use a VBO and upload a geometry and the full texture atlas of
> characters. I guess it doesn't matter just surprised to see it this way.

I generated with the software listed here:

http://www.codehead.co.uk/cbfg/

The conversion then to the byte array and the width array I mostly did manually. (i.e. some hacked on C++ code). I didn't even bother to save it sadly. It's probably worth readding it somewhere.
(In reply to Andreas Gal :gal from comment #6)
> Can you add the script that generated the png? And once thats loaded why
> don't you use a VBO and upload a geometry and the full texture atlas of
> characters. I guess it doesn't matter just surprised to see it this way.

Oh right, I missed this. So basically that would require me to implement VBs and such things in the Compositor API, and then implement them for all the backends. It wasn't impossible but it would've taken a lot more work that I would've needed to do in the different backends.

This was the easier road to getting the debug info we want.
Sure, its just debug anyway.
You need to log in before you can comment on or make changes to this bug.