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

3.x draft (for ci check only) #2204

Draft
wants to merge 16 commits into
base: dev
Choose a base branch
from
Draft

3.x draft (for ci check only) #2204

wants to merge 16 commits into from

Conversation

halx99
Copy link
Collaborator

@halx99 halx99 commented Oct 4, 2024

Describe your changes

  • Remove all deprecated stubs
  • Implement drawArraysInstanced and make instanced draw more common use
  • box2d-3.x
    • Make Box2dTest of cpp-tests works on box2d v3
    • Make Box2dTestBed of cpp-tests works on box2d v3
  • Remove chipmunk
    • Remove core/physics based on chipmunk
    • Remove ChipmunkTest from cpp-tests
    • Remove ChipmuniTestbed from cpp-tests
  • Change underlaying color type to Color4F aka ax::Color
  • Improve DrawNode, don't create backend::Buffer always
  • Reimplement core/physics with box2d v3, not all API compatible with axmol-2.x

Issue ticket number and link

Checklist before requesting a review

For each PR

  • Add Copyright if it missed:
    - "Copyright (c) 2019-present Axmol Engine contributors (see AUTHORS.md)."

  • I have performed a self-review of my code.

    Optional:

    • I have checked readme and add important infos to this PR.
    • I have added/adapted some tests too.

For core/new feature PR

  • I have checked readme and add important infos to this PR.
  • I have added thorough tests.

@halx99 halx99 changed the title [WIP] Remove deprecated stubs (for ci check only) [WIP] draft 3.x (for ci check only) Oct 4, 2024
@halx99 halx99 changed the title [WIP] draft 3.x (for ci check only) [WIP] 3.x draft (for ci check only) Oct 4, 2024
@halx99 halx99 changed the title [WIP] 3.x draft (for ci check only) 3.x draft (for ci check only) Oct 4, 2024
@smilediver
Copy link
Contributor

From outside it would be easier to track changes being made to 3.x branch if they all went through PRs like in the dev branch. They would also get CI check automatically, and there would be an opportunity for reviews and comments.

@halx99
Copy link
Collaborator Author

halx99 commented Oct 4, 2024

currently, 3.x is a draft branch or you can consider it is a temp branch, I don't like make PR one by one to waste much time. I create this PR is only for non-win32 platform ci-check, because I'm compile and test on my windows computer.

@aismann
Copy link
Contributor

aismann commented Oct 4, 2024

Physics should be an own test project like fairgui.
@halx99
What are you mean?

@halx99
Copy link
Collaborator Author

halx99 commented Oct 5, 2024

Physics should be an own test project like fairgui. @halx99 What are you mean?

I think physics test in cpp-tests is ok

@halx99 halx99 added this to the 3.0 milestone Oct 5, 2024
@aismann
Copy link
Contributor

aismann commented Oct 5, 2024

@halx99
Please have a look on the Performance Candy Mix test:

  • drawOrder: true it very slow (2.2 is faster) and closing get this:

Assertion failed!
Program: ..._GIT\axmol\build\bin\cpp-tests\Debug\cpp-tests.exe
File: D:__GIT\axmol\core\renderer\CustomCommand.cpp
Line: 221
Expression: _vertexBuffer

I getting this also on other tests when I switched drawOrder: to 'true'

@halx99
Copy link
Collaborator Author

halx99 commented Oct 5, 2024

@halx99 Please have a look on the Performance Candy Mix test:

  • drawOrder: true it very slow (2.2 is faster) and closing get this:

Assertion failed!

Program: ..._GIT\axmol\build\bin\cpp-tests\Debug\cpp-tests.exe File: D:__GIT\axmol\core\renderer\CustomCommand.cpp Line: 221

Expression: _vertexBuffer

image

Tested, works well, and no assertion

@aismann
Copy link
Contributor

aismann commented Oct 5, 2024

Click on this text plz: it switched the drawOrder
image
It raised if you switched back to false

here another example:
image

@halx99
Copy link
Collaborator Author

halx99 commented Oct 5, 2024

Click on this text plz: it switched the drawOrder image It raised if you switched back to false

here another example: image

Assertion failed was fixed.

@aismann
Copy link
Contributor

aismann commented Oct 5, 2024

Yeah. Works now.
Please have a look on the release comparing between 2.2/3.0 on Performance Candy Mix test again:
2.2 release version drawNode true:
Seems faster (on my machine >50 FPS always)
image

3.0 release version drawNode true:
image

And the drawing looks very bad like pixel jumps around whatever on Performance Candy Mix

@halx99
Copy link
Collaborator Author

halx99 commented Oct 5, 2024

On my machine, release build, both 2.2 & 3.0 is 60fps

@aismann
Copy link
Contributor

aismann commented Oct 5, 2024

On my machine, release build, both 2.2 & 3.0 is 60fps

Your machine is faster. You cant see this behavior.
What about the crazy drawing ? Looks it the same candy?

You have switch the drawOrder =>true (on the both release biulds)?

@halx99
Copy link
Collaborator Author

halx99 commented Oct 5, 2024

I also saw the crazy drawing

@aismann
Copy link
Contributor

aismann commented Oct 5, 2024

On my machine, release build, both 2.2 & 3.0 is 60fps

You have switch the drawOrder =>true (on the both release biulds)?

@halx99
Copy link
Collaborator Author

halx99 commented Oct 6, 2024

the ax::Color same with Color4F must be normalized

@aismann
Copy link
Contributor

aismann commented Oct 6, 2024

the ax::Color same with Color4F must be normalized

normalized What das this means?
ax::Color4F(1.0, 0, 0, 0.5) is also no alpha working

@halx99
Copy link
Collaborator Author

halx99 commented Oct 6, 2024

The ax::Color is just renamed from ax::Color4F, I don't understand what do mean no alpha working

@aismann
Copy link
Contributor

aismann commented Oct 6, 2024

@halx99
I checked it again. It works correct. Sorry.
I used ax:Color like ax::Color4B not like ax::Color4F

An assert >1.0f is useful?

@aismann
Copy link
Contributor

aismann commented Oct 12, 2024

I installed axmol on a new machine.
Compiling the 3.0 branch..
Starting the Box2D Testbed and ..boom....
image

Folder axmol\build\bin\cpp-tests\Debug\axslc\custom has no files:

 custom/circle_vs
 custom/circle_fs

@rh101
Copy link
Contributor

rh101 commented Oct 13, 2024

I installed axmol on a new machine.
Compiling the 3.0 branch..
Starting the Box2D Testbed and ..boom....

Given that this is a work in progress, it may be a little premature to be attempting to make use of it for any reason. Perhaps wait till it is out of draft and ready for testing.

@halx99
Copy link
Collaborator Author

halx99 commented Oct 13, 2024

I already add .fs .vs as glslcc shader file extension: https://github.com/axmolengine/axmol/blob/3.x/cmake/Modules/AXGLSLCC.cmake#L12

so I don't know why you meet the issue, maybe delete build folder, then re-cmake will fix your issue

@aismann
Copy link
Contributor

aismann commented Oct 13, 2024

so I don't know why you meet the issue, maybe delete build folder, then re-cmake will fix your issue

Works now: I download and rebuild axmol again.

@aismann
Copy link
Contributor

aismann commented Oct 13, 2024

[...] Perhaps wait till it is out of draft and ready for testing.

@rh101
You are right for Box2D 3.x stuff.
But this is all done and working for nativ using of Box2D 3.x
=> See the working Box2D tests on cpp-tests.
All other changes be more like PRs which we doing allways.
And of course this are also working well.

The only alpha status will be the internal physics, but this is not using be me at the moment.

Thats my opinium.

Edit:
Last but not least:
Testing this PR in an early state (parts which should work of course, not the alpha part!) permit/detect side effects earlier and can reduce time for the developer, because he needs not so much testing this parts. Of course mistakes done by the 'tester' can be raising too (thats the bad side).

@aismann
Copy link
Contributor

aismann commented Oct 22, 2024

@halx99
Please rename also all:
V2F_C4F_T2F_Triangle to V2F_T2F_C4F_Triangle
V3F_C4F_T2F_Quad to V3F_T2F_C4F_Quad
V2F_C4F_T2F_Quad to V2F_T2F_C4F_Quad

@smilediver
Copy link
Contributor

Why was vertex color changed from bytes to floats? This increases vertex size from 24 bytes to 36 by a 50%! Which will increase memory usage and traffic for no gain and at a performance cost, especially on mobile devices.

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.

5 participants