Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize StyleBoxFlat.draw() #94240

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

MewPurPur
Copy link
Contributor

@MewPurPur MewPurPur commented Jul 11, 2024

I'm not great at optimizing, but I needed this... I don't know which changes contributed most to the speedup, but resizing beforehand and the unfolding of the last nested loop seem like the primary two.

# Example of how I tested this
func _ready() -> void:
	var sbf := StyleBoxFlat.new()
	sbf.border_color = Color.RED
	sbf.set_corner_radius_all(4)
	var t := Time.get_ticks_msec()
	for i in 1000:
		sbf.draw(get_canvas_item(), Rect2(10, 10, 60, 20))
	print(Time.get_ticks_msec() - t)

Benchmarks:

  • Rounded stylebox: 205ms on master, 110ms in my PR (1.9x faster)
  • Stylebox with sharp corners: 75ms on master, 65ms in my PR (1.2x faster)

On production builds, on 4.3.beta2 I get 58ms on average, on this artifact I got 39ms on average.

This speedup should make stylebox drawing faster, for example if you draw hundreds of styleboxes in the same frame. It won't affect stylebox rendering after they have been drawn.

@MewPurPur MewPurPur force-pushed the optimize-styleboxflat branch 2 times, most recently from 5c16722 to aa89d1f Compare July 12, 2024 00:25
@Calinou Calinou added this to the 4.x milestone Jul 12, 2024
@Calinou
Copy link
Member

Calinou commented Jul 12, 2024

cc @davthedev

@MewPurPur MewPurPur force-pushed the optimize-styleboxflat branch from aa89d1f to 29767ff Compare July 12, 2024 00:26
@AThousandShips
Copy link
Member

I'd suggest using ptrw to improve iteration if we're resizing

@MewPurPur MewPurPur force-pushed the optimize-styleboxflat branch from 29767ff to f2dc57c Compare July 12, 2024 10:58
@MewPurPur
Copy link
Contributor Author

I'd suggest using ptrw to improve iteration if we're resizing

Not too familiar with pointers, but I think I got it? I got slightly better measures, but I don't think they are statistically-significant

@MewPurPur MewPurPur force-pushed the optimize-styleboxflat branch 3 times, most recently from 2cf9bec to 13902ca Compare July 12, 2024 14:33
@MewPurPur MewPurPur changed the title Optimize rounded StyleBoxFlat Optimize StyleBoxFlat.draw() Jul 12, 2024
@MewPurPur
Copy link
Contributor Author

MewPurPur commented Jul 12, 2024

Changed the title as I discovered a notable speedup in drawing styleboxes without rounded corners too.

@MewPurPur
Copy link
Contributor Author

I expected that some recent optimizations, like the Vector.insert() optimization, might change the benchmark. But I still measure the same 1.9x speedup for rounded styleboxes and 1.2x for all others.

@akien-mga akien-mga changed the title Optimize StyleBoxFlat.draw() Optimize StyleBoxFlat.draw() Sep 2, 2024
@akien-mga akien-mga requested review from Calinou and a team and removed request for Calinou September 2, 2024 09:13
@MewPurPur MewPurPur force-pushed the optimize-styleboxflat branch from 13902ca to 71ac6b7 Compare September 28, 2024 09:37
@MewPurPur MewPurPur requested a review from a team as a code owner September 28, 2024 09:37
Comment on lines +266 to +271
real_t quarter_arc_rad = Math_PI / 2.0;
Point2 style_rect_center = style_rect.get_center();

int colors_size = colors.size();
int verts_size = verts.size();
int new_verts_amount = (adapted_corner_detail + 1) * (draw_border ? 8 : 4);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
real_t quarter_arc_rad = Math_PI / 2.0;
Point2 style_rect_center = style_rect.get_center();
int colors_size = colors.size();
int verts_size = verts.size();
int new_verts_amount = (adapted_corner_detail + 1) * (draw_border ? 8 : 4);
const real_t quarter_arc_rad = Math_PI / 2.0;
const Point2 style_rect_center = style_rect.get_center();
const int colors_size = colors.size();
const int verts_size = verts.size();
const int new_verts_amount = (adapted_corner_detail + 1) * (draw_border ? 8 : 4);

For consistency.

Copy link
Member

@Geometror Geometror left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't benchmarked myself, but I believe you :) Great work!
While some aspects slightly impact readability, the performance boost is significant enough to justify the trade-off I think. I've played around with StyleBoxFlats for quite a bit and haven't observed any regressions.

verts.push_back(Vector2(x + x_skew, y + y_skew));
colors.push_back(color);
const real_t pt_angle = (corner_idx + detail / (double)adapted_corner_detail) * quarter_arc_rad + Math_PI;
const real_t angle_cosine = cos(pt_angle);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const real_t angle_cosine = cos(pt_angle);
const real_t angle_cosine = Math::cos(pt_angle);

colors.push_back(color);
const real_t pt_angle = (corner_idx + detail / (double)adapted_corner_detail) * quarter_arc_rad + Math_PI;
const real_t angle_cosine = cos(pt_angle);
const real_t angle_sine = sin(pt_angle);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const real_t angle_sine = sin(pt_angle);
const real_t angle_sine = Math::sin(pt_angle);

@Repiteo Repiteo modified the milestones: 4.x, 4.4 Sep 30, 2024
@akien-mga akien-mga merged commit eaac033 into godotengine:master Oct 1, 2024
19 checks passed
@MewPurPur MewPurPur deleted the optimize-styleboxflat branch October 1, 2024 17:06
@akien-mga
Copy link
Member

Belated thanks! And oops I merged by mistake, I intended to wait for style fixes first. Can be done in a follow-up :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants