Fix buttonset items' layout calculations and painting (fix #3554) #3565

martincapello · 2022-10-20T19:52:42Z

Fixes several issues when displaying buttonsets. This required a few changes in the sheet.png file.
To verify the changes see:

Context bar buttons.
Canvas size buttons.
Debugger buttons.
Outline buttons.
Dynamics buttons.
Symmetry buttons.
Pivot point options.

Fix #3554

martincapello · 2022-10-21T19:46:12Z

@Gasparoken Don't review this yet, because I will add some styles to make the buttonset a bit customizable. I will reassign you when done.

dacap · 2022-10-25T12:09:11Z

Just giving a try to this PR, the brush size button is not aligned with the center (I didn't see other buttons):

Previously:

martincapello · 2022-10-25T12:40:05Z

I can see the difference in the 1pixel case, but I don't see any difference with the 16px brush. Also, I think that any even brush size is displayed the same as before. For the odd cases I will investigate a bit, but I would suggest comparing how all the other buttonset items look now.

dacap · 2022-10-25T12:48:33Z

Sorry, the problem appears only with odd sizes:

martincapello · 2022-10-25T18:40:15Z

I will try to explain the issue using the canvas size buttons. This is how it looks with this PR:

In the image you can see the dimensions used to calculate the position of the icon inside each button. As you may notice, the last row of buttons look different than the others, despite using the same area for positioning elements. This is due to the following overlapping when painting:

In the above image I displaced a bit to the right one of the buttons in the last row to see how they overlap.

Then, for the Brush Type button in the context bar I use the same criteria to calculate the position of the icon:

Without this PR, the height used to calculate the position of the icon in the last row is greater than the other rows, then you actually end up having some misalignment issues like this:

A possible solution would be to modify a bit how the button set items are displayed to something like:

But, giving it a second thought, this may not work as well. Because the brush type button height would end up being 15px, and 15px / 2 = 7px = 14 px / 2. Currently the height used to calculate the position of the icon is the client bound height, which for the last row of buttons has 3 extra pixels, so currently is 16px for the brush type button, hence a 1px icon is positioned at 16px/2 = 8px, and this is the reason you see it aligned vertically (see EDIT2)

EDIT:
The horizontal alignment is displaced one pixel because of the change I made to getTextIconInfo(), currently it does:

box_x = (bounds.x + bounds.x2())/2 - box_w/2;

In this PR it does:

box_x = (bounds.x + bounds.x2() - box_w) / 2;

So, for the same 16px, in the first case box_x = 8px, and in the second case box_x = 7px. However in neither case it is aligned at the center, given that you cannot center 1px perfectly in a 16px row.

EDIT2:

But, giving it a second thought, this may not work as well. Because the brush type button height would end up being 15px, and 15px / 2 = 7px = 14 px / 2. Currently the height used to calculate the position of the icon is the client bound height, which for the last row of buttons has 3 extra pixels, so currently is 16px for the brush type button, hence a 1px icon is positioned at 16px/2 = 8px, and this is the reason you see it aligned vertically.

This is not right..this may actually work. I recall now that currently there is an -1px adjustment for the last row of buttons (or when the button has an icon), then it currently work because 16px/2 - 1px/2 = 8px and then it subtract 1px giving us 7px.
Also my calculations were wrong, they should have been: (15px - 1px)/2 = 7px which is not the same as (14px - 1px)/2 = 6px.
So, doing this

A possible solution would be to modify a bit how the button set items are displayed...

should work.

dacap · 2022-11-16T15:54:48Z

I'll take these changes from here. I saw some differences mainly with the width of buttons like this:

original:

with this patch:

martincapello · 2022-11-17T11:29:01Z

That's right. Without this patch, the last column of buttons in a buttonset are one pixel smaller in its width. Look at the 3 buttons after the lock in the original image you posted, the last one has a width 1 pixel smaller than the other two, which by the way have the same width that when using this patch.

dacap · 2022-11-17T12:41:09Z

I'll mark the buttons that seens a bit off:

I'll see what I do with this PR, at the moment just merged one commit to main which is unrelated to the PR. But then I'll see if it's better to merge it and redo some work (e.g. converting buttonset to styles) or starting from main again.

dacap · 2022-11-17T12:57:12Z

Some mockups of possible solutions:

Still not sure which one is better. I have to design a lot of details yet.

martincapello · 2022-11-17T13:01:27Z

The one at the top looks better IMO...I think that I could try to make it look like that with a minor change. I will try it and let you know.

…butes to styles

…are not a multiple of the gui scale

…ided by a widget

…string literal

…orrectly for big brushes As now the BrushType button has an odd number width (15px), it's better to limit the brush size to an odd number size (9px instead of 10px). In previous versions the BrushType button had an even number width (16px) so the 10px brush size was correctly centered.

The icon/stylus was inverted. We've also moved the icon for each state to the theme xml (so we don't need to use setIcon() manually anymore for this button).

…in tiles mode It was planned to use this color for this button when it's on.

Now these buttons look more like the previous version, where the special background color is painted to the edges. To achieve this the "buttonset_item_normal" part has less border to fill the background with "edit_pal_face" color in "pal_edit_button_unlock" and "edit_tiles_mode" styles.

…nsets We cannot use the fix from 6e2b44c as they contain different slice borders depending on the state (and that generated moving labels/icons in RGBA/Grayscale/Indexed buttons when we hover the mouse on them), so we had to revert it and use a new "buttonset_item_active" theme part to set the background of Edit Pal/Tiles.

We have to copy all missing <style> into the new theme, so these styles are re-loaded using colors and parts from the new theme (instead of using the data of the default theme).

martincapello requested a review from dacap as a code owner October 20, 2022 19:52

martincapello requested a review from Gasparoken October 20, 2022 19:52

martincapello assigned Gasparoken Oct 20, 2022

martincapello mentioned this pull request Oct 20, 2022

Test theme changes from #3519 with user themes #3555

Closed

martincapello force-pushed the fix-buttonset-issues branch 7 times, most recently from a56e5f0 to 0d98798 Compare October 21, 2022 18:33

martincapello unassigned Gasparoken Oct 21, 2022

martincapello assigned Gasparoken Oct 24, 2022

martincapello force-pushed the fix-buttonset-issues branch from 94361dc to 99d7531 Compare October 25, 2022 11:36

martincapello assigned dacap and unassigned Gasparoken Nov 9, 2022

dacap added the wip label Nov 16, 2022

dacap removed the wip label Nov 17, 2022

dacap assigned martincapello and unassigned dacap Nov 17, 2022

martincapello and others added 27 commits February 27, 2023 09:43

Take into account padding when painting text and icon layers

e002316

Add width, height, minwidth, minheight, maxwidth, and maxheight attri…

d3f01eb

…butes to styles

Fix positioning of text and icon layers to avoid setting values that …

1783704

…are not a multiple of the gui scale

Allow overriding the icon part used by icon layers with the icon prov…

5325f56

…ided by a widget

Add gap, gap-rows, and gap-columns style attributes

4042607

Update ButtonSet to use theme's styles

3ab993d

Initialize theme for HueSaturationWindow

cc3c421

Adjust styles to make them work with the new buttonset

ea43c5f

Add styles for ink type and dynamics buttons

029e520

Remove Y coordinate offset from some elements

0199994

Fix timeline's animation buttons size

f481d9c

Fix theme initialization for paint bucket settings menu

b2365b5

Fix theme initialization for theme variants widget

d88eb80

Add warning to ButtonSet::addItem() which is not safe to use w/a non-…

672f371

…string literal

Remove unneeded padding-top for brush_type styles

11ef2f9

Fix combobox height

9332642

Fix regression with Pal Edit button

5ea8914

The icon/stylus was inverted. We've also moved the icon for each state to the theme xml (so we don't need to use setIcon() manually anymore for this button).

Show the Tiles button highlighted ("button_selected_text") when it's …

0491bf6

…in tiles mode It was planned to use this color for this button when it's on.

Minor changes in theme.xml/style names

ff1211b

Minor change in formatting of conditional ternary operator

21a63b9

Don't use string literal in lambda inside ButtonSet::addItem()

c947047

Make buttons to select one window/multiple windows bigger

606fea4

Update dark theme

d95a021

Improve backward compatibility with old themes

0ae8bc9

We have to copy all missing <style> into the new theme, so these styles are re-loaded using colors and parts from the new theme (instead of using the data of the default theme).

martincapello force-pushed the fix-buttonset-issues branch from 6e1b3d8 to 0ae8bc9 Compare February 28, 2023 18:06

dacap merged commit 355ceac into aseprite:main Feb 28, 2023

martincapello deleted the fix-buttonset-issues branch October 10, 2023 18:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix buttonset items' layout calculations and painting (fix #3554) #3565

Fix buttonset items' layout calculations and painting (fix #3554) #3565

martincapello commented Oct 20, 2022 •

edited

Loading

martincapello commented Oct 21, 2022

dacap commented Oct 25, 2022

martincapello commented Oct 25, 2022

dacap commented Oct 25, 2022

martincapello commented Oct 25, 2022 •

edited

Loading

dacap commented Nov 16, 2022

martincapello commented Nov 17, 2022 •

edited

Loading

dacap commented Nov 17, 2022

dacap commented Nov 17, 2022 •

edited

Loading

martincapello commented Nov 17, 2022

Fix buttonset items' layout calculations and painting (fix #3554) #3565

Fix buttonset items' layout calculations and painting (fix #3554) #3565

Conversation

martincapello commented Oct 20, 2022 • edited Loading

martincapello commented Oct 21, 2022

dacap commented Oct 25, 2022

martincapello commented Oct 25, 2022

dacap commented Oct 25, 2022

martincapello commented Oct 25, 2022 • edited Loading

dacap commented Nov 16, 2022

martincapello commented Nov 17, 2022 • edited Loading

dacap commented Nov 17, 2022

dacap commented Nov 17, 2022 • edited Loading

martincapello commented Nov 17, 2022

martincapello commented Oct 20, 2022 •

edited

Loading

martincapello commented Oct 25, 2022 •

edited

Loading

martincapello commented Nov 17, 2022 •

edited

Loading

dacap commented Nov 17, 2022 •

edited

Loading