we’re using a fork of PlayCanvas for development of our product and after updating to latest release version we’ve noticed rendering errors as seen bellow:
We quickly identified the problematic commit.
But that one shouldn’t be in the release (and it actually isn’t, I’ll explain in a bit) as far as I can tell by looking at the release branch for release 1.61.3.
The problem we’re seeing is with the way we’re updating our fork and the way the release tags are created I guess.
What we usually do is checkout the release tag from upstream (e.g. v1.60.0), merge with our own changes and then merge into our main branch. And that’s usually fine but we’ve noticed now that these tags are not always on the release branch, they’re sometimes on main (e.g. our culprit v1.61.3).
When we tested with the released version Release v1.61.3 · playcanvas/engine (github.com) we noticed that version works fine as it doesn’t contain that commit and seems to be created from the release branch. But checking out the tag brings unwanted commits from main that are not on the release branch.
I wonder if this is a mistake on our tagging process or this tag.
main branch is considered to be unstable/cutting edge. It may have bugs etc because it hasn’t gone through our test plans.
When we are looking to do a minor release such as 1.60 to 1.61, we feature freeze the main branch, go through our test plan and create a release branch. In this case release-1.61.
This is released to the public as a NPM package and as a release candidate in the Editor for 1-2 weeks.
The release branch is then only updated with bug fixes that are cherry picked from main branch and then we release a patch after the test plan. This is released as a NPM package and also in the Editor.
TLDR, the release branches are considered to be stable and that’s what people should be using for production work.
I’m wondering if that tag was supposed to be on the release branch and was tagged incorrectly for that release. @mvaligursky@slimbuck, is this the case?
I’ll try to see if I can make a test project but main takeaway is that if we use a build of PlayCanvas from origin/release-1.61 release branch everything works fine but if we use a build of PC from main (which we get if we use this tag) that includes the commit that changes elements from using triangle fan to triangle strip then we get those issue probably because of the order of the vertices is expected to be different. We also have batching enabled for those elements.
Ah I see, the batching is likely the problem here, as it only supports triangle lists. This was triangle fan, and now is triangle strip, both of which are not supported (and it probably just worked in your case by accident).
I see, so the vertices just happened to be in the right order when they were batched and now with the switch to triangle strips it messes the vertices order. Does this mean that element items cannot be batched in general? What if they used indexed triangle list or just a simple triangle list, they’re just quads right?
Yep, we’d need to switch them to index triangle lists.
When I changed fans to strips in the mentioned PR (to make this work on WebGPU which does not support fans) I considered going to indexed triangles, as that’s more generic, but I’d have to handle index buffers, which were not used till now, so I avoid it that. I have not realised that batching would not work as I assumed it already didn’t, as fans are not supported.
I’ll investigate … maybe I can adjust batcher a bit to still handle strips by swapping order of vertices, or by converting it to triangle list at that point.
as I assumed it already didn’t, as fans are not supported.
