some uses of mDisplayPort in FrameMetrics are wrong now that we also have layer margins for display ports

RESOLVED FIXED in Firefox 30

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: tnikkel, Assigned: kats)

Tracking

29 Branch
mozilla31
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox29 unaffected, firefox30 fixed, firefox31 fixed, b2g-v1.3 unaffected, b2g-v1.4 fixed, b2g-v2.0 fixed)

Details

Attachments

(4 attachments, 1 obsolete attachment)

Posted patch PatchSplinter Review
This'll do for now but I'm going to file a follow-up bug to replace the use of FrameMetrics on the repaint code path with a different struct.
Assignee: nobody → bugmail.mozilla
Attachment #8401129 - Flags: review?(botond)
Attachment #8401129 - Flags: review?(botond) → review+
Not worth figuring out what the corrent thing to pass here is right now.
Attachment #8404083 - Flags: review?(botond)
Posted patch LogRenderTrace update (obsolete) — Splinter Review
Not too happy that we compute the rect even if we're not compiling with rendertrace enabled. Any C++ tricks that would make this nicer without #ifdef'ing stuff? I'm hoping the compiler might just optimize it away after inlining LogRendertraceRect but I don't know that it will since.
Attachment #8404084 - Flags: review?(botond)
Attachment #8404082 - Flags: review?(botond) → review+
Attachment #8404083 - Flags: review?(botond) → review+
Attachment #8404084 - Flags: review?(botond) → review+
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #5)
> Not too happy that we compute the rect even if we're not compiling with
> rendertrace enabled. Any C++ tricks that would make this nicer without
> #ifdef'ing stuff? I'm hoping the compiler might just optimize it away after
> inlining LogRendertraceRect but I don't know that it will since.

CSSRect GetDisplayPortRect()
{
    CSSRect baseRect(...);
    baseRect.Inflate(...);
    return baseRect;
}

...

LogRendertraceRect(GetGuid(), "requested displayport", "yellow", GetDisplayPortRect());
Updated, carrying r+
Attachment #8404084 - Attachment is obsolete: true
Attachment #8404102 - Flags: review+
Comment on attachment 8401129 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 957668
User impact if declined: things may be painted incorrectly at times. no specific STR but this is a follow-up patch for bug 957668
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): low-risk
String or IDL/UUID changes made by this patch: none
Attachment #8401129 - Flags: approval-mozilla-aurora?
Attachment #8404082 - Flags: approval-mozilla-aurora?
Attachment #8404083 - Flags: approval-mozilla-aurora?
Attachment #8404102 - Flags: approval-mozilla-aurora?
Attachment #8404102 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8404083 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8404082 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8401129 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
That didn't quite fix it. Backed out for now since I can't watch the tree much more. Will reland on Monday.

https://hg.mozilla.org/releases/mozilla-aurora/rev/cf8120f81e28
You need to log in before you can comment on or make changes to this bug.