Closed Bug 685326 Opened 8 years ago Closed 8 years ago

Add Round{In,Out} to BaseRect

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: joe, Assigned: joe)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Add BaseRect::Round{In,Out} (obsolete) — Splinter Review
Just like we have gfxRect::RoundIn and RoundOut, we should have BaseRect::RoundIn and RoundOut.
Attachment #558975 - Flags: review?(bas.schouten)
Assignee: nobody → joe
Comment on attachment 558975 [details] [diff] [review]
Add BaseRect::Round{In,Out}

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

I don't -really- care. It doesn't make a whole lot of sense for Int though. I'm fine with this, but asking Roc what he thinks.
Attachment #558975 - Flags: review?(roc)
Attachment #558975 - Flags: review?(bas.schouten)
Attachment #558975 - Flags: review+
It doesn't really matter to me where Round* goes; I only added it to BaseRect because the similar ScaleRound* functions were on it. Scale obviously would do something on IntRects, whereas Round wouldn't, but it seemed natural enough anyways.
Comment on attachment 558975 [details] [diff] [review]
Add BaseRect::Round{In,Out}

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

Seems OK. Shouldn't you remove RoundOut/RoundIn from gfxRect? Why not add Round() here too?

I think IntRect should declare RoundOut/RoundIn/Round and just define them to be noops.
Don't know precisely how I missed the fact that gfxRect is derived from BaseRect. Anyways, this removes the gfxRect versions, and adds an empty version to IntRect. These functions aren't virtual, but it doesn't matter since they should be no-ops on integers, and we always operate on the concrete class anyways.
Attachment #558975 - Attachment is obsolete: true
Attachment #558975 - Flags: review?(roc)
Attachment #559545 - Flags: review?(bas.schouten)
Bas, review ping?
Er, whoops - Bas wasn't cc'd.

Bas, review ping?
Comment on attachment 559545 [details] [diff] [review]
move Round{,In,Out} to BaseRect, remove it from gfxRect

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

r+ which change discussed on IRC.
Attachment #559545 - Flags: review?(bas.schouten) → review+
https://hg.mozilla.org/mozilla-central/rev/e6705f49eaaa
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in before you can comment on or make changes to this bug.