Better flat stylebox with rounded corners #8899

toger5 · 2017-05-24T21:19:43Z

This is still kind of WIP
NOW it's almost done though, it can be used but some features would still be nice to get added, check issues at the end of this post

what is supported for now:

UPDATED:

//Color
	void set_bg_color(const Color &p_color);
	Color get_bg_color() const;

	//Border Color
	void set_border_color_all(const Color &p_color);
	void set_border_color(Margin p_border, const Color &p_color);
	Color get_border_color(Margin p_border) const;

	void set_light_color(const Color &p_color);
	Color get_light_color() const;

	void set_dark_color(const Color &p_color);
	Color get_dark_color() const;

	//BORDER
	//width
	void set_border_width_all(int p_size);
	int get_min_border_width() const;

	void set_border_width(Margin p_margin, int p_size);
	int get_border_width(Margin p_margin) const;

	//blend
	void set_border_blend(bool p_blend);
	bool get_border_blend() const;

	//CORNER_RADIUS
	void set_corner_radius_all(int radius);
	void set_corner_radius_individual(const int radius_top_left, const int radius_top_right, const int radius_botton_right, const int radius_bottom_left);
	int get_min_corner_radius() const;

	void set_corner_radius(Corner p_corner, const int radius);
	int get_corner_radius(Corner p_corner) const;

	//EXPANDS
	void set_expand_margin_size(Margin p_expand_margin, float p_size);
	float get_expand_margin_size(Margin p_expand_margin) const;

	//FILLED
	void set_filled(bool p_draw);
	bool is_filled() const;

	//SHADOW
	void set_shadow_color(const Color &p_color);
	Color get_shadow_color() const;

	void set_shadow_size(const int &p_size);
	int get_shadow_size() const;

most of this is relatively easy to understand.
basically allows for rounded corners and different color for each corner.

how it works:

old version:
I draw rounded rectangles by first adding circles in the corners with the right offset and the right radius. than I draw four rectangles for each edge. (need for because it supports different radius for each corner).

I draw rounded rings by creating two arrays of certs which have a defined gap between them and fill them with polygons + set the color according to settings (if border_blend inner and outer arrays have different colors)
I also use that function to draw the shadow. with outer color = transparent
The aa will be done by creating another outer ring around the border and the center which will fade to transparent too.
The infill has other indexing logic but I also generate a vertex array in the shape of a rounded ring. just one though and than fill it with triangles from right to left.

issues:

toger5 · 2017-05-25T01:45:07Z

DEPRECATED!
I did another update.
updated the todoList.
Here are some screenshots of how it looks (just examples, I'm not claiming this makes the default theme look better) I think the flat design is kind of cool but some rounded corners might make it look better...
I think the biggest use case is in custom themes for games and other themes for the editor.

current version.

applying rounded corners to everything (older version -> broken ...)

how top left right bottom colors and border blend behave with rounded corners:

toger5 · 2017-05-25T01:54:08Z

@volzhs since you are very familiar with style boxes could you do some testing?
I wil try to keep this branch up to date, so that you can test with current master.

I can't change line width so I'm using multiple lines. I think performance wise the whole rounded style box thing is a desaster.. if someone could give some tips/fix some gels 3 functions to make it work would be highly appreciated.

toger5 · 2017-05-27T00:27:07Z

Fixed a couple of more things performance wise and added a couple of more features.

SirPigeonz · 2017-05-27T10:54:22Z

Yo, found few problems :)

Image worth 1000 words:

Things that may be unnoticed.
Props for borders color are float instead of color. Fill and border don't match resulting transparent pixels in between.

There are also problems when you modulate color to change Opacity. Original StyleBoxFlat also had problem with it ^_^'

toger5 · 2017-05-27T11:05:54Z

Here is a branch with some theme changes using rounded style boxes:
https://github.com/toger5/godot/tree/styleBoxAndThemeChanges
just clone and compile

toger5 · 2017-05-27T11:09:41Z

@n-pigeon Thank you!
the color, Int error should be fixed.

toger5 · 2017-05-27T11:35:51Z

@n-pigeon
the transparency bug say is not as easy to fix. I would need to build a vertex shape for the whole inner part so it would look good. (currently I'm drawing circles for the corners and rects for the center)

I also draw 10 lines max, reason is, that you can get it really buggy and slow if you allow more...

I think at the end I will have to go for full polygons anyways... damn that will be a little more work ;)

or I limit border radius to 10.. or 20 might still run fine.

too big of a limitation?

toger5 · 2017-05-27T12:34:22Z

rebased. on master

@akien-mga should I squash all the commits? or separate them in 1-3?
Since I rely on #8425, should I wait until this one is fixed anyways?
(of course this branch needs to get approved by u guys and tested properly but is still can start with squashing)

SirPigeonz · 2017-05-27T13:18:14Z

Full polygon will be probably needed sooner or later :) If you will need some help with this stuff I would gladly help.
Biggest issue will be AA. I have no idea how to make it :D

Zylann · 2017-05-27T16:23:36Z

@toger5 did you see my code comments?

toger5 · 2017-05-27T17:50:22Z

@Zylann sorry I did not. and I think the rebase removed them. let me check my mail if I still can get them.

-- also no email

Zylann · 2017-05-25T12:35:13Z

scene/resources/style_box.cpp

+		if (filled) {
+			//the circle has to be drawn twice, otherwise the circles for the corners are not visible
+			//super buggy ;)
+			vs->canvas_item_add_circle(p_canvas_item, corners[i], (float)corner_radius[i], col);


I think we could have a draw_filled_arc to divide polycount by 4.
Also drawing twice sounds like a bug to fix in rendering, we should keep track of it so that we revert it to one draw later

it is one draw now

filled arc is a really good idea!! also fixes some of the artefacts when drawing with transparency

Zylann · 2017-05-25T12:40:07Z

scene/resources/style_box.cpp

+			rad = 0;
+		inner_corner_radius[i] = rad;
+	}
+	int *p = &inner_corner_radius[0];


Wait, are you returning a pointer to a local here? Also why not using it internally to functions instead of expecting a pointer in others?

inline PoolIntArray get_offset_corner_radius(int offset, PoolIntArray corner_radius) { PoolIntArray inner_corner_radius = PoolIntArray(); for (int i = 0; i < 4; i++) { int rad = corner_radius[i] - offset; if (rad < 0) rad = 0; inner_corner_radius.append(rad); } return inner_corner_radius; }

is what I changed it to

well it's not that good either, you are allocating an array (which is also locking a mutex at every append) everytime you draw a corner. I don't understand why you created this function in the first place tbh, I feel like you don't really need it (or there must be a better way to prevent copy/paste)

I need to recalculate the corner radius in a couple of situations: drawing inner part of the box + drawing the border lines...
Should I allocate the memory inside the draw_rounded_rect function and just pass a ref to the function which modifies it?

My point is you probably don't need to allocate any memory^^ you could do that to an int[4] allocated on the stack. I see you use this function just to not copy/paste a reduction or increase in radius, so better make it so it's a zero-cost compression ;)
Also, some advice about PoolVectors: if you want to access them by index, whenever possible, use Read and Write objects, they will lock the array only once, resulting in faster access ;) (because using operator [] directly on the vector is slower if done repeatedly in a loop)

thank you for the explanation on read write objects. At the beginning I thought that would be the only way to use poolArray. than I found out you can use []... seemed so much simpler ;)
But now I know...

Zylann · 2017-05-25T12:44:18Z

scene/resources/style_box.cpp

+	ClassDB::bind_method(D_METHOD("get_corner_radius_TL"), &StyleBoxFlat::get_corner_radius_TL);
+	ClassDB::bind_method(D_METHOD("get_corner_radius_TR"), &StyleBoxFlat::get_corner_radius_TR);
+	ClassDB::bind_method(D_METHOD("get_corner_radius_BL"), &StyleBoxFlat::get_corner_radius_BL);
+	ClassDB::bind_method(D_METHOD("get_corner_radius_BR"), &StyleBoxFlat::get_corner_radius_BR);


naming convention should be lowercase

that makes a lot of sense since it also looks super wired in the inspector

Zylann · 2017-05-27T18:12:55Z

Looks like I forgot to click the "Submit review" button. I'm not used to this yet^^

toger5 · 2017-05-28T04:55:21Z

I was thinking that I might switch to a full polygon implementation. This results in better performance, cleaner code, better looks and also the option for border thickness.

border thickness would be great:
currently there is additional border size which was implemented by @volzhs, it works by just adding a rectangle to the desired border. It solves a similar problem than expand margin for style box texture. But expand margin just sizes the stylebox outside the boundaries.
For consistency I would like them to both have the same behaviour. With rounded corners thats needed anyways. And the implementation of border thickness would allow to achieve both with just expand margin.

SirPigeonz · 2017-05-28T17:41:35Z

I was thinking about ability to control thickness for all sides not just one global.

toger5 · 2017-05-28T18:01:33Z

Yea thats also what I was proposing
(bottom left in the sketch: border width per border)
It is a little chellanging how to decide on on corner radius and shape for inner and outer one without flooding the user with options. But I figured something out, just need to implement it.

I have decided now to reimplement the whole thing with polygons.

Hope ill find time soon

Zylann · 2017-05-28T18:50:02Z

What do you mean with polygons? Also I implemented a line mesh builder in scene/2d/ if you need one ;)

toger5 · 2017-05-28T18:59:46Z

@Zylann Currently I'm using draw rect, draw circle, draw line (of course the circle is using polygons too at the end) For the border I literally draw lines around the center... (got inspired by the super Hacky previous implementation of style box rendering: for the blend border multiple boxes were just drawn on top of each other with different sizes and colors)
I will calculate the vert array and color+ indices inside the draw function and than just do one draw call.
that is much easier to read, better performance, and less hacky.
I will checkout the line_mesh builder, but I already did the pseudocode for the whole poly shape with indexing+ color, infill... Maybe it's still easier with your line mesh builder.
With my implementation I can also do really easy shadows and fake anti aliasing. (anti aliasing by adding outer ring of verts with distance one to the box itself and than blend the colors to transparent)

volzhs · 2017-05-28T19:03:32Z

@toger5 I like that approach.

toger5 · 2017-05-29T02:47:15Z

Reimplemented the whole thing with polygons!
not feature complete yet:
still need shadows,
borders without out blending (currently everything is bending because I can't give individual indices colors).
The expand margin I proposed,
multiple border colors,
and some renaming of variables/bindings

toger5 · 2017-05-29T11:48:17Z

@volzhs I the branch with the poly implementation is uploaded. Ready for testing.

toger5 · 2017-05-29T12:25:20Z

This is the current naming for styleBoxFlat:

//Color
	void set_bg_color(const Color &p_color);
	Color get_bg_color() const;

	void set_all_border_color(const Color &p_color);

	void set_light_color(const Color &p_color);
	Color get_light_color() const;

	void set_dark_color(const Color &p_color);
	Color get_dark_color() const;

	//Border Color
	void set_border_color(const Color &p_color, const Margin &p_border);
	Color get_border_color(const Margin &p_border) const;

	void set_border_color_left(const Color &p_color);
	void set_border_color_top(const Color &p_color);
	void set_border_color_right(const Color &p_color);
	void set_border_color_bottom(const Color &p_color);
	Color get_border_color_left();
	Color get_border_color_top();
	Color get_border_color_right();
	Color get_border_color_bottom();

	//BORDER
	//blend
	void set_border_blend(bool p_blend);
	bool get_border_blend() const;
	
	//width
	void set_all_border_width(int p_size);
	int get_min_border_width() const;

	void set_border_width(Margin p_margin, int p_size);
	int get_border_width(Margin p_margin) const;

	void set_border_widht_left(int p_size);
	void set_border_widht_top(int p_size);
	void set_border_widht_right(int p_size);
	void set_border_widht_bottom(int p_size);
	int get_border_widht_left();
	int get_border_widht_top();
	int get_border_widht_right();
	int get_border_widht_bottom();
	
	//CORNER_RADIUS
	//set
	void set_all_corner_radius(int radius);
	PoolIntArray get_corner_radius() const;

	void set_corner_radius(const int radius_top_left, const int radius_top_right, const int radius_botton_right, const int radius_bottom_left);
	int get_min_corner_radius() const;

	void set_corner_radius_TL(int radius);
	void set_corner_radius_TR(int radius);
	void set_corner_radius_BR(int radius);
	void set_corner_radius_BL(int radius);
	int get_corner_radius_TL() const;
	int get_corner_radius_TR() const;
	int get_corner_radius_BR() const;
	int get_corner_radius_BL() const;

	void set_filled(bool p_draw);
	bool is_filled() const;

Thoughts?
what should I do with set_all...
should I just do overloaded functions for set_corner_radius ... one with one property and another one with margin + value and another one with four properties for all four corners, borders?

I renames set_draw_center to set_filled.
and border_size to border_width

SirPigeonz · 2017-05-29T21:56:45Z

I'm not sure about _radius_TL it should be at least _radious_tl.

Maybe _radius_t_l then it will be displayed in properties as: Radius T L.

toger5 · 2017-05-29T23:59:42Z

I did another big renaming. Can't get it to compile right now though: Some issue with bindings to ClassDB...
I got rid of all the _tl, _left functions. only leaving set_(margin) get_(margin) this is much better since the bidnigs allow to still use those functions for properties.
so in c++ you do: box.set_border_width(MARGIN_LEFT, 20);
in gdScript: box.border_width_left = 20
this tremendously reduces set, get functions in the style box and still allows for the same usability in gdScript

toger5 · 2017-08-04T14:38:34Z

@karroffel @akien-mga
The pr got rebased on current master again.
Tested and works well.

karroffel · 2017-08-07T21:50:32Z

Oh @toger5 there are conflicts now, can you resolve them? Then I'll merge :)

toger5 · 2017-08-07T23:49:32Z

Nice. Cant do it right now bu i will tomorrow!

toger5 · 2017-08-08T15:02:43Z

Although there are no conflicts anymore don't merge yet.
want to add documentation and remove light, dark color.

akien-mga · 2017-08-11T08:46:17Z

scene/resources/theme.cpp

-				sbflat->set_dark_color(dark);
-				sbflat->set_light_color(bright);
+				sbflat->set_border_color_all(bright);
+				//				sbflat->set_dark_color(dark);


If this is no longer relevant, best just remove it.

akien-mga · 2017-08-11T08:47:13Z

scene/resources/style_box.cpp

+		}
+		//DRAW OUTER BORDER AA
+		if (!(border_width[0] == 0 && border_width[1] == 0 && border_width[2] == 0 && border_width[3] == 0)) {
+			//			for (int i = 0; i < 4; i++) {


Please remove commented out code.

akien-mga · 2017-08-11T08:47:20Z

scene/resources/style_box.cpp

+			}
+		} else if (!(border_width[0] == 0 && border_width[1] == 0 && border_width[2] == 0 && border_width[3] == 0)) {
+			//DRAW INNER BORDER AA
+			//			for (int i = 0; i < 4; i++) {


akien-mga · 2017-08-11T08:49:21Z

core/math/math_2d.h

@@ -43,6 +43,14 @@ enum Margin {
 	MARGIN_BOTTOM
 };

+enum Corner {


If it's used only for StyleBoxFlat, should it really go in math_2d.h or could it be kept in style_box_flat.h?

I though it might could be used some where else so it does not hurt to give access to it by just importing math 2d...
But if you think it is improbable that it will be used seomwhere else I move it to style box

akien-mga · 2017-08-11T08:53:02Z

I'd prefer to have the two documentation commits in a separate PR after this one is merged. If there were big issues in this one and we wanted to partially or totally revert it, the documentation changes would make it very difficult (also if anyone changes the documentation before this PR is merged, you might have to rebase).

akien-mga · 2017-08-11T08:53:18Z

scene/resources/style_box.cpp


-	ClassDB::bind_method(D_METHOD("set_border_color", "margin", "color"), &StyleBoxFlat::set_border_color);
-	ClassDB::bind_method(D_METHOD("get_border_color", "margin"), &StyleBoxFlat::get_border_color);
+	//	ClassDB::bind_method(D_METHOD("set_border_color", "margin", "color"), &StyleBoxFlat::set_border_color);


Just get rid of comments, really.

akien-mga · 2017-08-11T08:53:25Z

scene/resources/style_box.cpp

-	ADD_PROPERTYI(PropertyInfo(Variant::COLOR, "border_color_bottom"), "set_border_color", "get_border_color", MARGIN_BOTTOM);
+	ADD_GROUP("Border", "border_");
+	ADD_PROPERTY(PropertyInfo(Variant::COLOR, "border_color"), "set_border_color", "get_border_color");
+	//	ADD_PROPERTYI(PropertyInfo(Variant::COLOR, "border_color_top"), "set_border_color", "get_border_color", MARGIN_TOP);


Get rid of those.

akien-mga · 2017-08-11T08:56:35Z

I still think there are too many commits for the overall changes. A rebase like this would be better IMO:

pick a129972	NEW Style Box Flat Implementation
pick 9585da8	Adapted godot to the new StyleBoxFlat	
pick 3fd82a9	Added Corner Enum
pick 8d2a439	new StyleBoxFlat added Corner Radius Bindings
squash 954d1fc	new StyleBoxFlat added Shadow
squash 192416b	new StyleBoxFlat added AntiAliasing
squash 90d8867	new StyleBoxFlat added Corner Detail
pick 2e0edae	new StyleBoxFlat limits for drawing
pick a08df42	new StyleBoxFlat removed multiBorderColor
squash 03681c1	styleBoxFlat removed light, dark color entirely
drop 5f252e8	Sync classes reference template with current code base
drop 177b1ef	documentation for StyleBoxFlat

toger5 · 2017-08-11T09:25:32Z

@akien ty for thw review!
Also your rebase proposal makes sense!
Dont know if i will have internet in the next two weeks.(holiday)
Will push the changes afterwards

- now use polygons! - renamed blend -> blend_border - draw_center -> filled - GDScript biding

- corner radius bindings - shadow - antiAliasing - CornerDetail

- corner and border are decreased if necassary to achoieve clean stylboxes - prohibits wired drawing artifacts when using wrong values - corner radius are relative to the partner corner when they would result in glitches

- removed only the bindings because the drawing code is not done yet - kept c++ functions for setting individual border color for future implementation

toger5 · 2017-08-15T17:56:07Z

@akien-mga rebased + squashed + removed comments.
the only thing I didn't do yet, is changing the location of the Corner declaration.
I commented on your review. What are your thoughts.

kubecz3k · 2017-08-15T23:17:49Z

from the meeting: @reduz approves but would like to hear also @karroffel and @akien-mga voice

neikeq added enhancement topic:core topic:rendering labels May 24, 2017

neikeq added this to the 3.0 milestone May 24, 2017

toger5 force-pushed the BetterFlatStylebox branch from ec5ca8c to f4fb105 Compare May 27, 2017 00:34

toger5 force-pushed the BetterFlatStylebox branch from 65c21df to 3fc41b7 Compare May 27, 2017 12:31

Zylann reviewed May 27, 2017

View reviewed changes

toger5 force-pushed the BetterFlatStylebox branch from 62f1965 to af51c7e Compare August 4, 2017 14:31

toger5 force-pushed the BetterFlatStylebox branch from af51c7e to 17d3058 Compare August 8, 2017 12:07

toger5 force-pushed the BetterFlatStylebox branch from 17d3058 to 177b1ef Compare August 8, 2017 16:32

akien-mga reviewed Aug 11, 2017

View reviewed changes

toger5 added 4 commits August 15, 2017 19:36

NEW Style Box Flat Implementation

f05e61e

- now use polygons! - renamed blend -> blend_border - draw_center -> filled - GDScript biding

Adapted godot to the new StyleBoxFlat

059a049

Added Corner Enum

c39ac86

new StyleBoxFlat added more drawing features

356eb47

- corner radius bindings - shadow - antiAliasing - CornerDetail

toger5 force-pushed the BetterFlatStylebox branch from 177b1ef to b7689d9 Compare August 15, 2017 17:49

toger5 added 2 commits August 15, 2017 19:49

new StyleBoxFlat limits for drawing

d637cbb

- corner and border are decreased if necassary to achoieve clean stylboxes - prohibits wired drawing artifacts when using wrong values - corner radius are relative to the partner corner when they would result in glitches

new StyleBoxFlat removed multiBorderColor

015494b

- removed only the bindings because the drawing code is not done yet - kept c++ functions for setting individual border color for future implementation

toger5 force-pushed the BetterFlatStylebox branch from b7689d9 to 701fb55 Compare August 15, 2017 18:11

styleBoxFlat removed light, dark color entirely

701fb55

akien-mga merged commit a8a6082 into godotengine:master Aug 16, 2017

groud mentioned this pull request Aug 21, 2017

Control border visual issue due to antialiasing #10461

Closed

Calinou mentioned this pull request May 18, 2023

Add styling possibilities to StyleBoxFlat + poly count optimization #77215

Open

Better flat stylebox with rounded corners #8899

Better flat stylebox with rounded corners #8899

Conversation

toger5 commented May 24, 2017 • edited Loading

what is supported for now:

how it works:

issues:

toger5 commented May 25, 2017 • edited Loading

toger5 commented May 25, 2017

toger5 commented May 27, 2017

SirPigeonz commented May 27, 2017

toger5 commented May 27, 2017

toger5 commented May 27, 2017

toger5 commented May 27, 2017

toger5 commented May 27, 2017

SirPigeonz commented May 27, 2017

Zylann commented May 27, 2017

toger5 commented May 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toger5 May 27, 2017 • edited Loading

Choose a reason for hiding this comment

Zylann May 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zylann May 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zylann commented May 27, 2017

toger5 commented May 28, 2017 • edited Loading

SirPigeonz commented May 28, 2017

toger5 commented May 28, 2017

Zylann commented May 28, 2017

toger5 commented May 28, 2017

volzhs commented May 28, 2017

toger5 commented May 29, 2017

toger5 commented May 29, 2017

toger5 commented May 29, 2017 • edited Loading

SirPigeonz commented May 29, 2017 • edited Loading

toger5 commented May 29, 2017

toger5 commented Aug 4, 2017

karroffel commented Aug 7, 2017

toger5 commented Aug 7, 2017

toger5 commented Aug 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akien-mga commented Aug 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akien-mga commented Aug 11, 2017

toger5 commented Aug 11, 2017

toger5 commented Aug 15, 2017

kubecz3k commented Aug 15, 2017

toger5 commented May 24, 2017 •

edited

Loading

toger5 commented May 25, 2017 •

edited

Loading

toger5 commented May 27, 2017 •

edited

Loading

toger5 May 27, 2017 •

edited

Loading

Zylann May 28, 2017 •

edited

Loading

Zylann May 28, 2017 •

edited

Loading

toger5 commented May 28, 2017 •

edited

Loading

toger5 commented May 29, 2017 •

edited

Loading

SirPigeonz commented May 29, 2017 •

edited

Loading