search

cocos2d / cocos2d-x

Cocos2d-x is a suite of open-source, cross-platform, game-development tools utilized by millions of developers across the globe. Its core has evolved to serve as the foundation for Cocos Creator 1.x & 2.x.
https://www.cocos.com/en/cocos2d-x
18.02k stars 7.05k forks source link

RichText._fontElements Why is the initialization size 20 instead of 0? #20741

Closed tidys closed 1 year ago

tidys commented 2 years ago

https://github.com/cocos2d/cocos2d-x/blob/cd7446f17956d73041f9b33b37b4c747c6bc1993/cocos/ui/UIRichText.cpp#L361

I see that properties are looked up in reverse order when they are fetched, and default values are returned when they are not found change pr: https://github.com/cocos2d/cocos2d-x/commit/a8ddbdc12c233c52b2746cd03e806d0fc59c2cdb

stevetranby commented 1 year ago

I think it's just an educated assumption that anyone using the UIRichText widget/class will on average have at least 20 different attributed text nodes in its content. I believe that this trades off a zero-initialized allocation at construction instead of resizing the std::vector a few times if it starts empty.

tidys commented 1 year ago

At the beginning I also thought it was pre-allocating some space, but then I saw that it was push and pop operations on top of 20, and if everything was normal, the first 20 elements of the array were not accessible

stevetranby commented 1 year ago

Ah, yeah it's very likely you're correct on that, but regardless it does makes sense to just initialize it to zero (0) because the UIRichText widget and associated MyXMLVisitor are used where performance is not critical.

And, to be honest, I'd be a bit surprised if UIRichText is used in many games.

Feel free to edit in github as a PR for the actual team to change this.

note: based on your comments, and taking a quick glance over the entire UIRichText.cpp source in github, I'm guessing the reason this is benign is that initializing 20 unused attributes probably does eliminate one or two resize growing allocations and each of these 20 default initialized UIRichText::Attributes instances have default values which then cause the each of the MyXMLVisitor::getXXXX functions to return a literal "default" value (ie: 12 in getFontSize). Therefore it's not an issue to set it as _fontElement(20), but it does make sense to change it back to 0 (or remove it to allow default init).

tidys commented 1 year ago

It is likely that this is the author of a handwritten 20, I tried to modify to 0, temporarily did not find any problems

qx commented 1 year ago

Don't waste time on this project