Cleanup layerrender

RESOLVED FIXED in Firefox 14

Status

()

RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: jrmuizel, Assigned: jrmuizel)

Tracking

unspecified
Firefox 14
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Comment hidden (empty)
(Assignee)

Comment 1

7 years ago
Created attachment 612561 [details] [diff] [review]
Remove needless try/catch block.

This was needed before, but I've since removed
the code that would throw.
Attachment #612561 - Flags: review?(bugmail.mozilla)
Comment on attachment 612561 [details] [diff] [review]
Remove needless try/catch block.

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

Does this actually impact performance? In generally the try/finally idiom is good for robustness but in this case I'm ok with taking it out if there's some sort of measurable impact.
Attachment #612561 - Flags: review?(bugmail.mozilla) → review+
(Assignee)

Comment 3

7 years ago
Created attachment 612660 [details] [diff] [review]
Unify pageRect retrieval

This ensures that we use the same pageRect for the whole drawing process and avoids the overhead of recreating it.
Attachment #612660 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 4

7 years ago
(In reply to Kartikaya Gupta (:kats) from comment #2)
> Comment on attachment 612561 [details] [diff] [review]
> Remove needless try/catch block.
> 
> Review of attachment 612561 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Does this actually impact performance? In generally the try/finally idiom is
> good for robustness but in this case I'm ok with taking it out if there's
> some sort of measurable impact.

No I would not expect this to have any impact on performance. I'm only taking it out because it seems unnecessary.
Comment on attachment 612660 [details] [diff] [review]
Unify pageRect retrieval

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

Cool. As future cleanup, I also think it's worthwhile to combine the "screen context" and "page context" into a single context. I think most of the fields are the same between the two, and that's been kind of confusing to me.
Attachment #612660 - Flags: review?(bugmail.mozilla) → review+
(In reply to Jeff Muizelaar [:jrmuizel] from comment #4)
> No I would not expect this to have any impact on performance. I'm only
> taking it out because it seems unnecessary.

Ok, I don't feel too strongly about it either way.
https://hg.mozilla.org/mozilla-central/rev/2b06484c231c
https://hg.mozilla.org/mozilla-central/rev/b295582d2c6a
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.