Closed Bug 422248 Opened 13 years ago Closed 13 years ago

[Mac] Buttons look bad since bug 418497

Categories

(Core :: Widget: Cocoa, defect, P2)

All
macOS
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: mstange, Assigned: vlad)

References

Details

(Keywords: testcase)

Attachments

(4 files, 1 obsolete file)

Bug 418497 changed the way of scaling non-natural sized buttons. Unfortunately, this has regressed button appearance at certain sizes. The
middle part of the button sometimes looks bad.

I see this on Mac OS 10.4 with button heights 17px, 20px and 24px. Empty buttons look funny, too.
Flags: blocking1.9?
Attached file Testcase
Attached patch patch (obsolete) — Splinter Review
Josh, this is essentially the patch that I sent you earlier -- it basically has us do an offscreen drawing that's the size of the native dimensions and then scale the image, instead of relying on the CGContext to do the scaling for us.  This sucks a little for some things that can draw themselves nicely scaled, but it'll do for now.
Assignee: joshmoz → vladimir
Status: NEW → ASSIGNED
Attachment #308764 - Flags: review?(joshmoz)
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
Please move the radio button data (NATURAL_MINI_RADIO_BUTTON_WIDTH declarations and radioButtonMargins) below the DrawCellWithScaling method so that it is right above the DrawRadioButton method.

==========
  else if (minWidth != 0.0f &&
           drawRect.size.width < minWidth)
  {
...
  } else {
==========

Opening bracket on the same line ("else {" vs. "else\n{") and newline before else ("\nelse {"). 

==========
    [cell drawWithFrame:drawRect inView:[NSView focusView]];

  } else {
==========

Extra newline below drawWithFrame

==========
CGContextDrawImage (cgContext, CGRectMake (drawRect.origin.x,
...
CGImageRelease (img);
CGContextRelease (ctx);
==========

We don't put space between the function name and its opening "(" for arguments, happens a few times there.

More coming.
Your patch also results in misdrawn radio buttons on bugzilla.
That radio button picture is from 10.5.
Attached patch updated patchSplinter Review
Ok, updated patch.

Also fixes a Leopard bug I caught -- the y-offset for checkboxes looks like is only needed on Tiger (at least, things render 1px too low on Leopard).

Radio buttons seem to suck a lot due to the NSCell not liking NSSmallControlSize; we always end up having to do the scaling for them.  Would be nice to figure out a workaround for this.
Attachment #308764 - Attachment is obsolete: true
Attachment #308958 - Flags: review?(joshmoz)
Attachment #308764 - Flags: review?(joshmoz)
Comment on attachment 308958 [details] [diff] [review]
updated patch

Looks good. This still remains, please change on checkin:

+  } else {
Attachment #308958 - Flags: review?(joshmoz) → review+
Attachment #308958 - Flags: superreview?(pavlov) → superreview+
Duplicate of this bug: 422701
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
verified fixed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b5pre) Gecko/2008032615 Firefox/3.0b5pre ID:2008032615
Status: RESOLVED → VERIFIED
Keywords: testcase
You need to log in before you can comment on or make changes to this bug.