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

Mention current GLES2 rendering bugs in the Skeleton2D class #41927

Closed

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Sep 9, 2020

See #35772.

@Calinou Calinou requested a review from a team as a code owner September 9, 2020 20:03
@Calinou Calinou force-pushed the doc-skeleton2d-rendering-bugs branch from 16c4e05 to 59088b8 Compare September 9, 2020 20:03
@Calinou Calinou added this to the 3.2 milestone Sep 9, 2020
@clayjohn
Copy link
Member

clayjohn commented Sep 9, 2020

I'm not sure we should be documenting bugs in this case. 3.2.3 will come with a few fixes for Skeleton2D.

IMO we should only document bugs for features which are unlikely to receive fixes.

@Calinou
Copy link
Member Author

Calinou commented Sep 9, 2020

@clayjohn I'm not sure if 3.2.3 will fix Skeleton2D rendering fully. A more comprehensive fix may only come for 3.2.4, which is a few months away.

@clayjohn
Copy link
Member

clayjohn commented Sep 9, 2020

I don't think 3.2.3 will fix all the problems either. However, I still think that the only bugs we should document are the ones that we will definitely not fix before 4.0.

@kjav
Copy link
Contributor

kjav commented Sep 10, 2020

I've closed my issue as this supercedes it. I do think that the documentation should also mention that 2d skeletons are also broken for GLES3; see this comment #35772 (comment)
Also GLES3 doesn't work on many android phones; so this solution is about as bad as the original problem, as it only affects some phones.

@clayjohn I really think this issue should be documented. It is frustrating to use this class in an Android game only to find out that it doesn't work, and there is no way a user would be expected to know that this feature currently doesn't work in the current release, and the upcoming release, of godot.

@Calinou
Copy link
Member Author

Calinou commented Oct 27, 2020

We ended up merging godotengine/godot-docs#3989. Should we also merge this PR for consistency?

@akien-mga
Copy link
Member

I'm also not fond of documenting bugs in the class reference, but I leave the decision to folks that actually understand the potential impact of the current bugs and the expectations (or lack thereof) for future fixes.

If bugs are the result of design limitations that we can't easily lift, we can indeed mention it to set expectations straight.

@lawnjelly
Copy link
Member

lawnjelly commented Nov 16, 2020

Note that in 3.2, 2d skeletons are now fixed, as the default is now software skinning in GLES2 and GLES3 as of 3.2.4, so this is in a state of flux as far as the docs are concerned.

There is actually still one small deviation in behaviour between hardware skinning and software, which an offset is applied to the skeleton node itself or a node above a polygon .. such that the skeleton and the polygon move in relation to each other. But as far as I can see the software skinning is still usable.

I'm waiting for more feedback from beta testers to determine whether this is a problem in reality on any projects. (I spent a day debugging this but no joy, the hardware path is quite different and it may need the author of the hardware skinning to determine whether this is problematic, I'm not even 100% sure whether the hardware skinning behaviour is 'correct').

We could alternatively keep hardware skinning as a default, but that would probably lead to more confusion, as the software skinning is far more compatible on all hardware, which is important for exports. Once software skinning has been properly beta tested, we could put a note in that hardware skinning should be used with care only when it proves to have greater performance, and that compatibility will be more limited. In many cases software skinning will probably be faster anyway because it can be batched.

We may end up removing hardware skinning in 2d entirely, reduz did mention this might be worth doing in 4.0 when we discussed it, as the low number of polys in 2d skinning don't typically benefit much from the GPU anyway.

@Calinou
Copy link
Member Author

Calinou commented Dec 9, 2020

Closing, as the 2D skeleton rendering bug should now be fixed on Android starting from Godot 3.2.4. See also godotengine/godot-docs#4449.

@Calinou Calinou closed this Dec 9, 2020
@Calinou Calinou removed this from the 3.2 milestone Dec 9, 2020
@Calinou Calinou deleted the doc-skeleton2d-rendering-bugs branch March 31, 2021 09:46
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.

5 participants