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 DrawNode #2165

Merged
merged 5 commits into from
Sep 21, 2024
Merged

Optimize DrawNode #2165

merged 5 commits into from
Sep 21, 2024

Conversation

smilediver
Copy link
Contributor

This PR does some DrawNode optimizations:

  • Disabled transform by default, which had significant performance impact by default.
  • Use pod_vector for storing and growing buffers.
  • Don't update actual vertex buffer every time drawXXX() is called.
  • Avoid unnecessary allocations on some drawXXX() methods in _transform().

There's still a lot of drawXXX() methods that do unnecessary allocations that have a big impact on performance.

Also there are _lineWidth and _defaultLineWidth members that can be set by the user but do nothing. Seems like a bug that probably breaks old behavior.

@halx99 halx99 added this to the 2.2.0 milestone Sep 19, 2024
@aismann
Copy link
Contributor

aismann commented Sep 20, 2024

There's still a lot of drawXXX() methods that do unnecessary allocations that have a big impact on performance.

@smilediver
Can you add comments like // To do: Improve: unnecessary allocations ... or better can you improve this methods too?

Also there are _lineWidth and _defaultLineWidth members that can be set by the user but do nothing. Seems like a bug that probably breaks old behavior.

See here:
DrawNode: setLineWitdh() is no longer working#1411
See here for more info:
#1411 (comment)

@halx99 @smilediver:
setLineWidth() should be deprecated.

@smilediver
Copy link
Contributor Author

Can you add comments like // To do: Improve: unnecessary allocations ... or better can you improve this methods too?

It would be better if you did that, since you wrote it and know the code better.

setLineWidth() should be deprecated.

Maybe it's better to remove it completely, since it's not doing anything anyway, and will mislead people.

@aismann
Copy link
Contributor

aismann commented Sep 20, 2024

Can you add comments like // To do: Improve: unnecessary allocations ... or better can you improve this methods too?

It would be better if you did that, since you wrote it and know the code better.

setLineWidth() should be deprecated.

Maybe it's better to remove it completely, since it's not doing anything anyway, and will mislead people.

I will look after merge.

@halx99 halx99 merged commit 5c31ecb into axmolengine:dev Sep 21, 2024
15 checks passed
@halx99
Copy link
Collaborator

halx99 commented Sep 21, 2024

@aismann
Copy link
Contributor

aismann commented Sep 23, 2024

@smilediver (@halx99)
Can you explain me what this means:
bool _trianglesDirty: 1 = false;

@aismann
Copy link
Contributor

aismann commented Sep 23, 2024

@aismann could we delete https://github.com/axmolengine/axmol/tree/dev/extensions/DrawNodeEx/src/DrawNodeEx?

I read to late, yes can deleted.

@smilediver
Copy link
Contributor Author

Can you explain me what this means: bool _trianglesDirty: 1 = false;

It's a bit field: https://en.cppreference.com/w/cpp/language/bit_field

@smilediver smilediver deleted the optimize_draw_node branch October 4, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants