1278204 - [jsplugins] DrawTextAt should support HiDPI case

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Evelyn Hung]

When you run a Flash on a high DPI device(i.e device pixel ratio > 1), the text is unclear.

I tried to enlarge canvas' width and height like this:

        ratio = devicePixelRatio / backingStoreRatio;
        canvas.width = oldWidth * ratio;
        canvas.height = oldHeight * ratio;
        canvas.style.width = oldWidth + 'px';
        canvas.style.height = oldHeight + 'px';
        context.scale(ratio, ratio);

but it doesn't work in our case, because we do pixel manipulation before/after drawing text on the canvas.

Comment 1

[Michael Bebenita [:mbx]]

I don't know the context here. How does the canvas relate to the Flash player? To get sharp text rendering I'm guessing you need to let the Flash player know about the devicePixelRatio somehow.

Comment 2

[Yury Delendik (:yury)]

See also http://www.adobe.com/devnet/air/articles/high_resolution_images_flash_player.html and http://help.adobe.com/en_US/FlashPlatform/reference/actionscript/3/flash/display/Stage.html#contentsScaleFactor . Notice that this functionality is available in the Flash Player v 11.5 (and content/movie shall target this version as well -- SWF version 18)

Comment 3

[Benjamin Smedberg]

Evelyn could you be more specific about your testcase/steps to reproduce? I can't figure out how to triage this bug.

Comment 4

[Peter Van der Beken [:peterv]]

This is a bug related to the Pepper implementation.

Comment 5

[Evelyn Hung]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #3)
> Evelyn could you be more specific about your testcase/steps to reproduce? I
> can't figure out how to triage this bug.

DrawTextAt is a Pepper API call to ask browser draw a text. We use canvas API to paste this text on a bitmap image background, but the text looks blur in HiDPI case.

Peter, would you like to take this bug as you said you have figured it out?

Comment 6

[Evelyn Hung]

Attachment: wip-hidpi-from-peterv.patch

A WIP patch from Peter.

Comment 7

[Evelyn Hung]

Attachment: Screen Shot 2016-09-07 at 3.59.00 PM.png

After applying the patch, text displaying is off position.

It seems we didn't set line height correctly. (Actually, we return a fixed value in the |metrics| field when the plugin ask for PPB_BrowserFont_Trusted::Describe a font. Need to figure out a way to calculate correct values.


Log of our implementation:
[Main Thread]: V/PPAPIJSParent From plugin: {"__interface":"PPB_BrowserFont_Trusted","__version":"1.0","__method":"Describe","font":10,"description":140734540468048,"metrics":59725655466240}

[Main Thread]: V/PPAPIJSParent RPC response from plugin: [[1,{"description":{"face":{"type":0,"padding":0,"value":{"as_bool":0}},"family":2,"size":22,"weight":3,"italic":0,"small_caps":0,"letter_spacing":0,"word_spacing":0,"padding":1989},"metrics":{"height":12,"ascent":12,"descent":12,"line_spacing":12,"x_height":12}}]]

[Main Thread]: V/PPAPIJSParent From plugin: {"__interface":"PPB_BrowserFont_Trusted","__version":"1.0","__method":"DrawTextAt","font":10,"image_data":6,"text":{"text":{"type":"PP_VARTYPE_STRING","padding":0,"value":{"as_bool":"???","as_int":3,"as_double":1.4822e-323,"as_id":3}},"rtl":"PP_FALSE","override_direction":"PP_FALSE"},"position":{"x":74,"y":254}, "color”:4278190 080,"clip":{"point":{"x":74,"y":242},"size":{"width":204,"height":24}}, "image_data_is_opaque":"PP_FALSE"}

Log from Chrome:
{"__interface":"PPB_BrowserFont_Trusted","__version":"1.0","__method":"Describe","font":690,"description":140734724004496,"metrics":9501314269056}

RPC response: [["PP_TRUE",{"description":{"face":{"type":"PP_VARTYPE_STRING","padding":0,"value":{"as_bool":"???","as_int":1759,"as_double":8.69061e-321,"as_id":1759}},"family":"PP_BROWSERFONT_TRUSTED_FAMILY_MONOSPACE","size":22,"weight":"PP_BROWSERFONT_TRUSTED_WEIGHT_400","italic":"PP_FALSE","small_caps":"PP_FALSE","letter_spacing":0,"word_spacing":0,"padding":1},"metrics":{"height":25,"ascent":20,"descent":5,"line_spacing":25,"x_height":11}}]]

{"__interface":"PPB_BrowserFont_Trusted","__version":"1.0","__method":"DrawTextAt","font":690,"image_data":66,"text":{"text":{"type":"PP_VARTYPE_STRING","padding":0,"value":{"as_bool":"???","as_int":1767,"as_double":8.73014e-321,"as_id":1767}},"rtl":"PP_FALSE","override_direction":"PP_FALSE"},"position":{"x":74,"y":262},"color":4278190080,"clip":{"point":{"x":74,"y":242},"size":{"width":204,"height":25}},"image_data_is_opaque":"PP_FALSE"}

Comment 8

[Evelyn Hung]

The work on bug 1102584 will help here to get rich TextMetrics information from canvas.

Comment 9

[Evelyn Hung]

Attachment: HiDPI.patch

update the patch against current master.

Comment 10

[Evelyn Hung]

I tried the patch in bug 1102584 and it basically works. However I found the information exposed in TextMetrics isn't enough for solving my problem. I need line_spacing and x_height but it seems they are not accessible in Canvas APIs.

Hi Jonathan, do we have a way that I can get these information in chrome privileged JavaScript? Thanks.

Comment 11

[Jonathan Kew [:jfkthame]]

(In reply to Evelyn Hung [:evelyn] from comment #10)
> I tried the patch in bug 1102584 and it basically works. However I found the
> information exposed in TextMetrics isn't enough for solving my problem. I
> need line_spacing and x_height but it seems they are not accessible in
> Canvas APIs.
> 
> Hi Jonathan, do we have a way that I can get these information in chrome
> privileged JavaScript? Thanks.

Not that I am aware of, sorry. :(  

This sounds kinda like stuff that would belong in the proposed Font Metrics API (https://drafts.css-houdini.org/font-metrics-api/), but (a) that's not yet standardized or implemented, and (b) even there, I don't see x_height being exposed at all. ()

Note, though, that AFAICS the Font Metrics API is about measuring what happens for a specific piece of text + style (which may involve font fallback, etc.) rather than about querying one specific font for its values. Which of those are you needing here? "Tell me the line_height and x_height of font X" or "Tell me the overall line_height and x_height of text 'fooبار' in style-context Y"?

Comment 12

[Evelyn Hung]

(In reply to Jonathan Kew (:jfkthame) from comment #11) 
> Note, though, that AFAICS the Font Metrics API is about measuring what
> happens for a specific piece of text + style (which may involve font
> fallback, etc.) rather than about querying one specific font for its values.
> Which of those are you needing here? "Tell me the line_height and x_height
> of font X" or "Tell me the overall line_height and x_height of text 'fooبار'
> in style-context Y"?

Thanks for the quick response! 

My case is a bit weird, at this step I do need "tell me the line_height and x_height
of font X". There are a few steps in my case to draw a text on screen.

1. The plugin requests browser to create a Font object with a font style preference.

2. Browser checks if we do have this font support, otherwise, fallback to a default font style. Return a font object id to the plugin.

3. The plugin requests browser to *Describe* the font object in terms of font style and metrics. (so it can layout the to-be-drawn text properly.)

4. Browser needs to respond this request. <-- This is the step where I'm in, but at this moment I have no idea what text is going to be drawn.

5. The plugin requests browser to measure the width of a certain text by passing the text string and a font object id (usually the one it just requested to create).

6. Browser response the width of the text.

7. The plugin requests to *DrawTextAt* a certain text on screen by given position, size, range, text, and font object id.

8. Browser draws the text on screen.

In comment 7, you can see the log for step 3, 4 and 7.

Comment 13

[Jonathan Kew [:jfkthame]]

(In reply to Evelyn Hung [:evelyn] from comment #12)

> 1. The plugin requests browser to create a Font object with a font style
> preference.

Does this "font style preference" specify a single font-family name, or does it involve a CSS font-family property (which may contain a list of family names)?

> 2. Browser checks if we do have this font support, otherwise, fallback to a
> default font style. Return a font object id to the plugin.

This font ID would presumably correspond to an nsFontMetrics struct, roughly speaking...

> 3. The plugin requests browser to *Describe* the font object in terms of
> font style and metrics. (so it can layout the to-be-drawn text properly.)
> 
> 4. Browser needs to respond this request. <-- This is the step where I'm in,
> but at this moment I have no idea what text is going to be drawn.

...but nsFontMetrics is not currently exposed to script in any way, AFAIK; it's part of the internals of the interface between layout and gfx.

What exactly is the "font object id" you're dealing with in step 2 here -- can you point me at the code responsible for this? Once we understand what this is, we may be able to figure out how to get it to provide the metrics you need.

Comment 14

[Evelyn Hung]

(In reply to Jonathan Kew (:jfkthame) from comment #13)
> (In reply to Evelyn Hung [:evelyn] from comment #12)
> 
> > 1. The plugin requests browser to create a Font object with a font style
> > preference.
> 
> Does this "font style preference" specify a single font-family name, or does
> it involve a CSS font-family property (which may contain a list of family
> names)?
> 

No, it won't be a list of family names. It's an option from a predefined set of font-families (DEFAULT, SERIF, SANSSERIF, MONOSPACE), or a customized string (font name) which the plugin presumes the browser supported this font name. (At the very beginning of launching, the plugin queries to know all font names that the browser/system has.)


> > 2. Browser checks if we do have this font support, otherwise, fallback to a
> > default font style. Return a font object id to the plugin.
> 
> This font ID would presumably correspond to an nsFontMetrics struct, roughly
> speaking...
>

Hmm... I think the ID is irrelevant to the discussion, apologize for mentioning it without a clear explanation. The ID is just a serial number to manage created objects for Pepper API resources, so both sides(browser/plugin) know which resource of this Pepper API call refers to, without passing redundant arguments back and forth. We don't actually bind this ID to anything other than the Pepper API objects.
 
> > 3. The plugin requests browser to *Describe* the font object in terms of
> > font style and metrics. (so it can layout the to-be-drawn text properly.)
> > 
> > 4. Browser needs to respond this request. <-- This is the step where I'm in,
> > but at this moment I have no idea what text is going to be drawn.
> 
> ...but nsFontMetrics is not currently exposed to script in any way, AFAIK;
> it's part of the internals of the interface between layout and gfx.
> 

Then I think I should create a way to get these internal information, shouldn't I?
Or if there are any values exposed to JS/CSS which I can approximately calculate these values, I can try and see if it's good enough.

Thanks for your valuable input!

Comment 15

[Jonathan Kew [:jfkthame]]

(In reply to Evelyn Hung [:evelyn] from comment #14)
> No, it won't be a list of family names. It's an option from a predefined set
> of font-families (DEFAULT, SERIF, SANSSERIF, MONOSPACE), or a customized
> string (font name) which the plugin presumes the browser supported this font
> name. (At the very beginning of launching, the plugin queries to know all
> font names that the browser/system has.)

OK. Be aware, though, that the set of available fonts can change on the fly, so this API needs to be prepared to handle the case where a requested font name is no longer present. But it looks like your step (2) allows for that (fallback to default if necessary).


> > > 4. Browser needs to respond this request. <-- This is the step where I'm in,
> > > but at this moment I have no idea what text is going to be drawn.
> > 
> > ...but nsFontMetrics is not currently exposed to script in any way, AFAIK;
> > it's part of the internals of the interface between layout and gfx.
> > 
> 
> Then I think I should create a way to get these internal information,
> shouldn't I?
> Or if there are any values exposed to JS/CSS which I can approximately
> calculate these values, I can try and see if it's good enough.

I can't think of anything that is currently exposed in a way that would be useful for you. :( The extended canvas TextMetrics object (bug 1102584) and the proposed Font Metrics API (mentioned in comment 11) are both kind-of-related to this, but they're more about measurements based on a specific run of text, which may involve font fallback (perhaps to multiple fonts), not purely querying one specific font. (And neither of them are currently available, anyway.)

Ultimately, when you draw text I guess you're going to end up instantiating a gfxFont (or a mozilla::2d::ScaledFont or something like that) -- and that will be the same font object as eventually gets used to render the text. That font should be able to provide metrics in some way, though we might need to make some currently-internal APIs more public, depending how everything fits together. Where's the code that handles your steps (7)-(8) above, and what platform-level drawing APIs does that end up using?

Comment 16

[Evelyn Hung]

Attachment: Bug 1278204 - Fix HiDPI issue by detecting devicePixelRatio.

Review commit: https://reviewboard.mozilla.org/r/120534/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/120534/

Comment 17

[Evelyn Hung]

Louis, I decided to fix jsplugin part for PDF. Although it will cause text off position in Flash, but I'd like to file follow-up bug for Flash and move this bug forward. Thanks.

Comment 18

[Evelyn Hung]

(In reply to Evelyn Hung [:evelyn] from comment #17)
> ... but I'd like to file follow-up bug for Flash and move this bug forward.

Bug 1347502 filed for Flash.

Comment 19

[Louis Chang [:louis]]

Comment on attachment 8847557 [details]
Bug 1278204 - Fix HiDPI issue by detecting devicePixelRatio.

LGTM.

Comment 20

[Louis Chang [:louis]]

Comment on attachment 8847557 [details]
Bug 1278204 - Fix HiDPI issue by detecting devicePixelRatio.

https://reviewboard.mozilla.org/r/120534/#review122846

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb68333457ad
Fix HiDPI issue by detecting devicePixelRatio. r=lochang

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cb68333457ad