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

[v3.8.5] Reduce package size for spine module #17783

Merged
merged 82 commits into from
Nov 5, 2024

Conversation

dumganhar
Copy link
Contributor

@dumganhar dumganhar commented Oct 29, 2024

Re: #17715

■3.8.4
-rw-r--r-- 1 james staff 653581 Oct 11 14:08 spine.asm.js
-rw-r--r-- 1 james staff 57346 Oct 11 14:08 spine.js.mem
-rwxr-xr-x 1 james staff 351821 Oct 11 14:08 spine.wasm
-rw-r--r-- 1 james staff 56455 Oct 11 14:08 spine.wasm.js
■3.8.5
-rw-r--r--@ 1 james staff 377837 Nov 1 06:36 spine.asm.js ( save 42% )
-rw-r--r--@ 1 james staff 19563 Nov 1 06:36 spine.js.mem ( save 65%)
-rwxr-xr-x@ 1 james staff 198881 Nov 1 06:36 spine.wasm ( save 43%)
-rw-r--r--@ 1 james staff 21944 Nov 1 06:36 spine.wasm.js (save 61%)

Changelog


Continuous Integration

This pull request:

  • needs automatic test cases check.

    Manual trigger with @cocos-robot run test cases afterward.

  • does not change any runtime related code or build configuration

    If any reviewer thinks the CI checks are needed, please uncheck this option, then close and reopen the issue.


Compatibility Check

This pull request:

  • changes public API, and have ensured backward compatibility with deprecated features.
  • affects platform compatibility, e.g. system version, browser version, platform sdk version, platform toolchain, language version, hardware compatibility etc.
  • affects file structure of the build package or build configuration which requires user project upgrade.
  • introduces breaking changes, please list all changes, affected features and the scope of violation.

@@ -39,6 +39,7 @@ namespace spine {
class BoneData;

class SP_API IkConstraintData : public ConstraintData {
RTTI_DECL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why need it?

Copy link
Contributor Author

@dumganhar dumganhar Nov 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we don't want the default c++ rtti support which will make the package size 20KB bigger.
Spine has already provoided RTTI strategy, so we use RTTI_DECL/RTTI_IMPL to mark the classes that need the RTTI information. RTTI information is used in getting a unique type id for a instance which like c++ typeid(*pointer).

With the RTTI information, a c++ function return a Base pointer which points a Sub instance will be downcast to Sub JS object in emscripten layer. Otherwise, without the RTTI info, in JS layer, we only could see Base class.

// c++

class Base {};

class Sub {};

class Foo {
    Base* getBase() { return new Sub(); }
};

// js

const instance = foo.getBase(); // it's a Base instance without RTTI info, but in fact, it should be seen as a `Sub` instance.

}
RTTI_IMPL_NOPARENT(Interpolation)
RTTI_IMPL(PowInterpolation, Interpolation)
RTTI_IMPL(PowOutInterpolation, Interpolation)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why need it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explained above.


#define ENABLE_EMBIND_TEST 0

#if ENABLE_EMBIND_TEST
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add some embind test and disable it by default.

@@ -51,7 +51,7 @@ Atlas::Atlas(const String &path, TextureLoader *textureLoader, bool createTextur
const char *lastSlash = lastForwardSlash > lastBackwardSlash ? lastForwardSlash : lastBackwardSlash;
if (lastSlash == path) lastSlash++; /* Never drop starting slash. */
dirLength = (int)(lastSlash ? lastSlash - path.buffer() : 0);
dir = SpineExtension::calloc<char>(dirLength + 1, __FILE__, __LINE__);
dir = SpineExtension::calloc<char>(dirLength + 1, __SPINE_FILE__, __SPINE_LINE__);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SPINE_FILE is defined to empty string for emscripten build. That will save some bytes.

_a *= _skeleton.getScaleX();
_b *= _skeleton.getScaleX();
_c *= _skeleton.getScaleY();
_d *= _skeleton.getScaleY();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor Bone::updateWorldTransform, save some directives by invoking the getter at the beginning of the function.

}

void Bone::setWorldY(float inValue) {
_worldY = inValue;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the implementation of getter/setter to header and inline them.
Inlining them could make wasm/asmjs package size smaller and improve performance a little bit.

@@ -43,8 +43,6 @@ class SP_API Constraint : public Updatable {

virtual ~Constraint();

virtual void update() = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update function was already defined in its parent class Updatable. No need to define it here again.

@@ -45,8 +45,6 @@ class SP_API CurveTimeline : public Timeline {

virtual void apply(Skeleton& skeleton, float lastTime, float time, Vector<Event*>* pEvents, float alpha, MixBlend blend, MixDirection direction) = 0;

virtual int getPropertyId() = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getPropertyId is defined in Timeline, doesn't need to define the pure virtual function here again.

result |= (c - 'a' + 10);
}
}
return true;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A simpler version of sscanf(ptr + 1, "%4x", &uc); which could make the package size smaller.

@@ -378,15 +415,15 @@ const char *Json::parseNumber(Json *item, const char *num) {
}

while (*ptr >= '0' && *ptr <= '9') {
exponent = (exponent * 10.0) + (*ptr - '0');
exponent = (exponent * 10) + (*ptr - '0');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*ptr is a value between 0 and 9, exponent could only be an integer. So define it as uint32_t and use integer version of pow funciton (MathUtil::ipow) could make the package size smaller again.

float MathUtil::pow(float a, float b) {
return (float)::pow(a, b);
}
uint64_t MathUtil::ipow(uint64_t base, uint32_t exp) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add an integer version of pow function. It's simpler and higher performance than the builtin float version.


namespace spine {

class SP_API MathUtil : public SpineObject {
class SP_API MathUtil {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MathUtil doesn't need to be implemented from 'SpineObject'.

@dumganhar
Copy link
Contributor Author

@cocos-robot run test cases

bool same = other._buffer == _buffer;
_buffer = SpineExtension::realloc(_buffer, _length + 1, __FILE__, __LINE__);
memcpy((void *)(_buffer + thisLen), (void *)(same ? _buffer : other._buffer), len + 1);
return *this;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving most methods of spine::String to .cpp to avoid inline too many code.

darkColor.r += (darkColor.r - slot._data.getDarkColor().r) * alpha;
darkColor.g += (darkColor.g - slot._data.getDarkColor().g) * alpha;
darkColor.b += (darkColor.b - slot._data.getDarkColor().b) * alpha;
color.r += (color.r - dataColor.r) * alpha;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't invoke getColor, getDarkColor many times.

@@ -72,14 +75,14 @@ class SP_API Vector : public SpineObject {
return _size;
}

inline void setSize(size_t newSize, const T &defaultValue) {
void setSize(size_t newSize, const T &defaultValue) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove inline keyword of function that contains many lines. Let compiler deside whether it should be inlined.

@@ -164,6 +167,12 @@ class SP_API Vector : public SpineObject {
return _buffer[inIndex];
}

inline const T &operator[](size_t inIndex) const {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add const version of [] accessor.

Copy link

github-actions bot commented Nov 5, 2024

@dumganhar, Please check the result of run test cases:

Task Details

Copy link

github-actions bot commented Nov 5, 2024

@dumganhar, Please check the result of run test cases:

Task Details

@dumganhar dumganhar merged commit 102d05e into cocos:v3.8.5 Nov 5, 2024
26 checks passed
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