Last Comment Bug 685326 - Add Round{In,Out} to BaseRect
: Add Round{In,Out} to BaseRect
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla11
Assigned To: Joe Drew (not getting mail)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-07 15:09 PDT by Joe Drew (not getting mail)
Modified: 2012-02-01 14:00 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Add BaseRect::Round{In,Out} (1.87 KB, patch)
2011-09-07 15:09 PDT, Joe Drew (not getting mail)
bas: review+
Details | Diff | Splinter Review
move Round{,In,Out} to BaseRect, remove it from gfxRect (7.04 KB, patch)
2011-09-09 12:57 PDT, Joe Drew (not getting mail)
bas: review+
Details | Diff | Splinter Review

Description Joe Drew (not getting mail) 2011-09-07 15:09:25 PDT
Created attachment 558975 [details] [diff] [review]
Add BaseRect::Round{In,Out}

Just like we have gfxRect::RoundIn and RoundOut, we should have BaseRect::RoundIn and RoundOut.
Comment 1 Bas Schouten (:bas.schouten) 2011-09-07 19:27:13 PDT
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.
Comment 2 Joe Drew (not getting mail) 2011-09-07 19:57:07 PDT
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 3 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-09-07 20:12:43 PDT
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.
Comment 4 Joe Drew (not getting mail) 2011-09-09 12:57:13 PDT
Created attachment 559545 [details] [diff] [review]
move Round{,In,Out} to BaseRect, remove it from gfxRect

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.
Comment 5 Joe Drew (not getting mail) 2011-10-07 22:19:05 PDT
Bas, review ping?
Comment 6 Joe Drew (not getting mail) 2011-10-07 22:21:48 PDT
Er, whoops - Bas wasn't cc'd.

Bas, review ping?
Comment 7 Bas Schouten (:bas.schouten) 2011-10-12 13:40:02 PDT
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.
Comment 8 Ed Morley [:emorley] 2011-11-14 19:37:21 PST
https://hg.mozilla.org/mozilla-central/rev/e6705f49eaaa

Note You need to log in before you can comment on or make changes to this bug.