422248 - [Mac] Buttons look bad since bug 418497

Status: VERIFIED FIXED

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

Description

[Markus Stange [:mstange]]

Attachment: Screenshot showing rendering differences

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.

Comment 1

[Markus Stange [:mstange]]

Attachment: Testcase

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: patch

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.

Comment 3

[Josh Aas]

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.

Comment 4

[Josh Aas]

Attachment: misdrawn radio buttons, pic 1

Your patch also results in misdrawn radio buttons on bugzilla.

Comment 5

[Josh Aas]

That radio button picture is from 10.5.

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: updated patch

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.

Comment 7

[Josh Aas]

Comment on attachment 308958 [details] [diff] [review]
updated patch

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

+  } else {

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Checked in.

Comment 10

[Carsten Book [:Tomcat]]

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