Bug 109428 - [texmap] TextureMapper::draw{Border,RepaintCounter} should reuse textures
Summary: [texmap] TextureMapper::draw{Border,RepaintCounter} should reuse textures
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Bruno Abinader (history only)
URL:
Keywords:
Depends on: 107942 109540
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-11 06:57 PST by Bruno Abinader (history only)
Modified: 2013-02-13 19:18 PST (History)
4 users (show)

See Also:


Attachments
Exploratory patch (TextureMapper::createNumberAtlas) (4.80 KB, patch)
2013-02-13 18:54 PST, Bruno Abinader (history only)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bruno Abinader (history only) 2013-02-11 06:57:45 PST
TextureMapper debug visuals-related functions should reuse textures whenever possible, specially when drawing FPS values on-screen (handled in bug 107942).
Comment 1 Bruno Abinader (history only) 2013-02-11 20:27:54 PST
I've started thinking about a nice approach for this and came with two different ideas:

1. Add a RefPtr<BitmapTextureGL> m_fpsTexture to TextureMapperGL which would be updated if the repaint/frame count changes (also including additional info like color, position and transformation matrix). A separate drawFPS() function would be then necessary to differentiate from the repaint count texture.

2. FPS counter could be handled on its own compositing layer, so it could could paint itself into a separate backing surface to avoid unnecessary repaints.

The latter solution sounds less hacky, however might add an unnecessary footprint. The former is easier to implement, but requires an additional BitmapTexture that shall live outside of BitmapTexturePool's scope. What do you guys think?
Comment 2 Dongseong Hwang 2013-02-11 20:53:51 PST
(In reply to comment #1)
> I've started thinking about a nice approach for this and came with two different ideas:
> 
> 1. Add a RefPtr<BitmapTextureGL> m_fpsTexture to TextureMapperGL which would be updated if the repaint/frame count changes (also including additional info like color, position and transformation matrix). A separate drawFPS() function would be then necessary to differentiate from the repaint count texture.
> 
> 2. FPS counter could be handled on its own compositing layer, so it could could paint itself into a separate backing surface to avoid unnecessary repaints.
> 
> The latter solution sounds less hacky, however might add an unnecessary footprint. The former is easier to implement, but requires an additional BitmapTexture that shall live outside of BitmapTexturePool's scope. What do you guys think?

Both idea are good, but I have another idea.

First of all, I'm not sure if we need to optimize drawBorder, which is already fast.

When it comes to drawRepaintCounter (FYI, I tried to rename it to draw Number in Bug 109540), how about using texture atlas (refer to http://code.google.com/p/freetype-gl/).
we need to draw only 1,2,3,4,5,6,7,8,9. So if we pre-build number texture, we can draw number quickly for both: repaint count and fps.

What do you guys think?
Comment 3 Bruno Abinader (history only) 2013-02-12 08:04:51 PST
(In reply to comment #2)
> (In reply to comment #1)
> Both idea are good, but I have another idea.
> 
> First of all, I'm not sure if we need to optimize drawBorder, which is already fast.
> 
> When it comes to drawRepaintCounter (FYI, I tried to rename it to draw Number in Bug 109540), how about using texture atlas (refer to http://code.google.com/p/freetype-gl/).
> we need to draw only 1,2,3,4,5,6,7,8,9. So if we pre-build number texture, we can draw number quickly for both: repaint count and fps.
> 
> What do you guys think?

Sounds good, better than saving textures each time the value changes. Would clipping the border/number bounding box area out of the render layer tree (i.e. make it an opaque layer in front of all other RenderLayers, so they wouldn't need to render anything below this) also help?
Comment 4 Bruno Abinader (history only) 2013-02-13 18:54:56 PST
Created attachment 188248 [details]
Exploratory patch (TextureMapper::createNumberAtlas)

This patch creates a number atlas GL texture using BitmapTextureGL. Do you guys agree on this approach?
Comment 5 Dongseong Hwang 2013-02-13 19:18:53 PST
(In reply to comment #4)
> Created an attachment (id=188248) [details]
> Exploratory patch (TextureMapper::createNumberAtlas)
> 
> This patch creates a number atlas GL texture using BitmapTextureGL. Do you guys agree on this approach?

good progress!
you absolutely create number texture but it is not number texture atlas, because we can not get rect of each number on texture unit.
you need to draw each number using TextRun and store the geometry info using your own data structure.
See GraphicsContext::drawBidiText to know TextRun.