Support "object-fit" & "object-position" on <canvas>

RESOLVED FIXED in mozilla36

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

(Blocks: 1 bug, {dev-doc-complete})

Trunk
mozilla36
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
Created attachment 8527016 [details]
testcase 1

Just realized that we should probably support "object-fit" & "object-position" on HTML5 <canvas>.  Right now, they have no effect on <canvas>, but they probably should, since it's a replaced element. (They also do have an effect in Chrome.)

Testcase attached.
 - Works in Chrome 41 (renders as a circle, respecting "object-fit")
 - Doesn't work in Firefox Nightly. (stretches to fill the <canvas>'s CSS width/height)
Keywords: dev-doc-needed
(Assignee)

Updated

4 years ago
Depends on: 1104354
(Assignee)

Comment 1

4 years ago
Created attachment 8528644 [details] [diff] [review]
fix v1

This should do it, I think. This fixes the attached testcase. (This requires the GetIntrinsicSize impl that I'm adding in bug 1104354, too.)

Just requesting feedback instead of review at the moment, because I want to test more thoroughly & add reftests. (It's likely done or near-done, though.)

For reference:
 - the "GetOpaqueRegion" changes are similar to the nsImageFrame changes that I had to make in bug 1101128 (but they're bit simpler here, because nsImageFrame can be fragmented but nsHTMLCanvasFrame cannot).

 - the BuildLayer changes here are very similar to the nsVideoFrame::BuildLayer changes that I made over in bug 624647 -- here's a link to those changes, for reference:
https://hg.mozilla.org/integration/mozilla-inbound/rev/466d3ff030e6#l3.29

(BTW: I also realized today that we can simplify those^ nsVideoFrame changes by keeping around the old "area" local-var. I've got a trivial followup patch to land as soon as the tree reopens that does that. In the meantime, this patch here uses the existing 'area' rect, instead of exactly matching the nsVideoFrame changeset.)
Attachment #8528644 - Flags: feedback?(roc)
(Assignee)

Updated

4 years ago
Blocks: 1105150
(Assignee)

Comment 2

4 years ago
Created attachment 8528813 [details] [diff] [review]
fix v2

Just ended up needing one more change -- need to pass the right clip flags (& not necessarily assume that drawing will be constrained to the content-box).

(I'm using the same nsStyleUtil::ObjectPropsMightCauseOverflow() helper-method that I added/used for this over in Bug 624647, in mozilla-central changeset a083cd9a7c8c )
Attachment #8528644 - Attachment is obsolete: true
Attachment #8528644 - Flags: feedback?(roc)
Attachment #8528813 - Flags: review?(roc)
(Assignee)

Comment 3

4 years ago
Created attachment 8528814 [details] [diff] [review]
part 2: reftests (using modified copies of existing tests w/ <object>)

Posting the tests as a separate patch.  They're just modified copies of existing tests (plus the script I used to do the modifications, for archival sake), so I'm not bothering with review on them -- but feedback is welcome, of course!
https://hg.mozilla.org/mozilla-central/rev/052e9649b6ba
https://hg.mozilla.org/mozilla-central/rev/b5c23a7a6d8c
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Target Milestone: mozilla37 → mozilla36
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.