[haiku-commits] Re: haiku: hrev47085 - src/kits/interface
- From: John Scipione <jscipione@xxxxxxxxx>
- To: "haiku-commits@xxxxxxxxxxxxx" <haiku-commits@xxxxxxxxxxxxx>
- Date: Fri, 4 Apr 2014 19:30:19 -0400
On Fri, Apr 4, 2014 at 6:06 PM, Ingo Weinhold <ingo_weinhold@xxxxxx> wrote:
> On 04/04/2014 01:55 AM, John Scipione wrote:
>> Ingo Weinhold wrote:
>>> On 03.04.2014 22:42, John Scipione wrote:
>> For the tab key in Keymap, which is too low, see here:
>>
>> http://31.media.tumblr.com/7a2dacf9378cdf83b7ee1c53a44a0d45/tumblr_n3h99rkffx1r0f0hfo1_500.png
>>
>> I got the following numbers:
>>
>> rect.height: 9.999998
>> font.ascent: 5.654602
>> font.descent: 1.436703
>
> When I print the values, I get:
>
> KeyboardLayoutView::_DrawKey(): TAB key
> rect: (-0.000010, 88.586952) - (27.999990, 106.586952)
> BControlLook::DrawLabel(): TAB
> rect: (3.999990, 92.586952) - (23.999990, 102.586952)
> rect.Height(): 10.000000
> ascent: 12.924804
> descent: 3.283893
> textHeight: 17.000000
> alignedRect: (9.999990, 92.586952) - (16.999990, 102.586952)
> location: (9.999990, 105.586952)
>
> The text height is significantly larger than the available height.
Thank you for taking the time to run these numbers.
I'm a bit confused why our ascent and descent numbers disagree. It
appears that you printed your numbers within ControlLook::DrawLabel()
while I printed my from KeyboardLayoutView::_DrawKey(), but, I would
think they should be the same. Assuming that your numbers are correct,
then I think we've identified the problem.
> As a work-around for old/broken code, a boolean parameter could be added to
> BLayoutUtils::AlignInFrame() which would force alignment in case of a
> constraint violation. E.g.:
>
> BRect
> BLayoutUtils::AlignInFrame(BRect frame, BSize maxSize,
> BAlignment alignment, bool forceAlignment = false)
> {
> // align according to the given alignment
> float delta = frame.Width() - maxSize.width;
> if ((delta > 0 && alignment.horizontal != B_ALIGN_USE_FULL_WIDTH)
> || (delta < 0 && forceAlignment)) {
> frame.left += int(delta * alignment.RelativeHorizontal());
> frame.right = frame.left + maxSize.width;
> }
> [...]
> }
>
> Using it with forceAlignment = true in this case would do the negative
> alignment that was done before.
No, that is not very elegant, I don't like that solution at all. You
say that it is a work-around for old/broken code, but, what code is
old/broken exactly? I'd rather keep the current code than alter
BLayoutUtils::AlignInFrame() in such a manner.
>> http://24.media.tumblr.com/ae6711c88880d7cbe993b0f86e071288/tumblr_n3h72l1mwW1r0f0hfo1_400.png
>>
>> Both images are set to 10pt DejaVu Sans font, the buttons in the top
>> image are set to a 3px margin, the buttons in the bottom image have no
>> margin. As you can see the margin cuts off the text in some of the
>> buttons making the app unusable.
>
> Someone decided that a margin -- more correctly: spacing between button
> frame and text -- of 3 pixels produces aesthetically pleasing results.
It is the wisdom of this decision that I am calling into question. In
a previous email you stated that including a margin, or, in CSS
parlance, padding in BButton::Draw() increases the button size, so
omitting the padding makes all buttons smaller. I thought about this
some more and this isn't the case.
Insetting the drawing area before drawing the label in BButton::Draw()
has no effect on the button size. In order to achieve that you need to
alter the MinSize(), MaxSize(), or PreferredSize() methods to include
a margin, or, in the case of BButton that has a frame rectangle
specified (the old way), the ResizeToPreferred() method.
I have no problem specifying a padding in those methods, either at a
fixed size like we currently do, or as a settable parameter. However,
if you don't use the layout APIs to layout your buttons, and you don't
call ResizeToPreferred() but instead you specify the button positions
manually as is done in BDH Calc you should be allowed to utilize the
total button area to display the label, and we can only achieve this
if we remove the Inset line from BButton::Draw().
> If you force the button to a smaller size than it needs, then, obviously,
> something will break visually. Currently only the text will be truncated
> while the margin stays intact. By centering the text in the rect that
> includes the margin, the margin will break and potentially (when small
> enough) the text as well.
Forcing a padding in Draw() breaks things visually as you say, but,
there is no gain for this breakage. As I am trying to explain above,
we can have our cake and eat it too.
> If old code relies on the latter behavior, it's probably best to use that
> behavior. New code simply shouldn't violate the button's size constraints. A
> setter for the margins would address special needs.
Having padding on buttons is visually pleasing, but, it doesn't make
sense where it currently is in the Draw() method. I hope I've made
myself clear and haven't made a mistake in my above logic.
Other related posts:
