From b62d421d8d185a896f26b13688e4feff831ffbd2 Mon Sep 17 00:00:00 2001 From: Kimmo Hoikka Date: Fri, 26 Oct 2018 19:19:20 +0100 Subject: [PATCH] Fix a bug in AnimatablePropertyRegistration with default value method did not recurse base classes so if default value was asked from derived class, it returned wrong value Change-Id: I29691a911e77bd9431863590560104792e57fc07 --- automated-tests/src/dali/utc-Dali-CustomActor.cpp | 99 ++++++++++++++++++----- dali/internal/event/common/object-impl.cpp | 6 +- dali/internal/event/common/type-info-impl.cpp | 7 +- 3 files changed, 88 insertions(+), 24 deletions(-) diff --git a/automated-tests/src/dali/utc-Dali-CustomActor.cpp b/automated-tests/src/dali/utc-Dali-CustomActor.cpp index 761f969..f6d0b34 100644 --- a/automated-tests/src/dali/utc-Dali-CustomActor.cpp +++ b/automated-tests/src/dali/utc-Dali-CustomActor.cpp @@ -1345,6 +1345,8 @@ int utcDaliActorGetTypeInfo(void) END_TEST; } +namespace Impl +{ /** * A custom actor that is not type registered on purpose */ @@ -1395,51 +1397,49 @@ struct UnregisteredCustomActor : public Dali::CustomActorImpl virtual void OnLayoutNegotiated( float size, Dimension::Type dimension ) { } }; - -struct UnregisteredCustomActorHandle : public Dali::CustomActor +} +struct UnregisteredCustomActor : public Dali::CustomActor { - static UnregisteredCustomActorHandle New() + static UnregisteredCustomActor New() { - UnregisteredCustomActor* impl = new UnregisteredCustomActor; - UnregisteredCustomActorHandle custom( *impl ); // takes ownership + Impl::UnregisteredCustomActor* impl = new Impl::UnregisteredCustomActor; + UnregisteredCustomActor custom( *impl ); // takes ownership return custom; } - UnregisteredCustomActorHandle() - { - } - UnregisteredCustomActorHandle( Internal::CustomActor* impl ) + UnregisteredCustomActor() + { } + ~UnregisteredCustomActor() + { } + UnregisteredCustomActor( Internal::CustomActor* impl ) : CustomActor( impl ) - { - } - UnregisteredCustomActorHandle( UnregisteredCustomActor& impl ) + { } + UnregisteredCustomActor( Impl::UnregisteredCustomActor& impl ) : CustomActor( impl ) + { } + static UnregisteredCustomActor DownCast( BaseHandle handle ) { - } - static UnregisteredCustomActorHandle DownCast( BaseHandle handle ) - { - UnregisteredCustomActorHandle hndl; + UnregisteredCustomActor hndl; CustomActor custom = Dali::CustomActor::DownCast( handle ); if( custom ) { CustomActorImpl& customImpl = custom.GetImplementation(); - UnregisteredCustomActor* impl = dynamic_cast(&customImpl); + Impl::UnregisteredCustomActor* impl = dynamic_cast( &customImpl ); if( impl ) { - hndl = UnregisteredCustomActorHandle(customImpl.GetOwner()); + hndl = UnregisteredCustomActor( customImpl.GetOwner() ); } } return hndl; } - }; int UtcDaliCustomActorSetGetActorPropertyActionSignal(void) { TestApplication application; // Need the type registry - auto custom = UnregisteredCustomActorHandle::New(); + auto custom = UnregisteredCustomActor::New(); Stage::GetCurrent().Add( custom ); // should have all actor properties @@ -1471,7 +1471,7 @@ int UtcDaliCustomActorSetGetActorPropertyActionSignal(void) DALI_TEST_EQUALS( Vector3( 100.0f, 150.0f, 200.0f ), custom.GetProperty( Actor::Property::POSITION ).Get(), TEST_LOCATION ); DALI_TEST_EQUALS( Vector3( 100.0f, 150.0f, 200.0f ), custom.GetCurrentPosition(), TEST_LOCATION ); - Dali::WeakHandle weakRef( custom ); + Dali::WeakHandle weakRef( custom ); // should have actor signals custom.ConnectSignal( &application, "offStage", [weakRef]() @@ -1482,6 +1482,63 @@ int UtcDaliCustomActorSetGetActorPropertyActionSignal(void) Stage::GetCurrent().Remove( custom ); Stage::GetCurrent().Add( custom ); + END_TEST; +} + +namespace Impl +{ +struct DerivedCustomActor : public UnregisteredCustomActor +{ }; +} + +struct DerivedCustomActor : public UnregisteredCustomActor +{ + static DerivedCustomActor New() + { + Impl::DerivedCustomActor* impl = new Impl::DerivedCustomActor; + DerivedCustomActor custom( *impl ); // takes ownership + return custom; + } + DerivedCustomActor() + { } + ~DerivedCustomActor() + { } + DerivedCustomActor( Internal::CustomActor* impl ) + : UnregisteredCustomActor( impl ) + { } + DerivedCustomActor( Impl::UnregisteredCustomActor& impl ) + : UnregisteredCustomActor( impl ) + { } +}; + +// register custom +DALI_TYPE_REGISTRATION_BEGIN( DerivedCustomActor, UnregisteredCustomActor, nullptr ); +DALI_TYPE_REGISTRATION_END() + +int UtcDaliCustomActorPropertyRegistrationDefaultValue(void) +{ + TestApplication application; // Need the type registry + + // register our base and add a property with default value for it + Dali::TypeRegistration typeRegistration( typeid( UnregisteredCustomActor ), typeid( Dali::CustomActor ), nullptr ); + + auto derived = DerivedCustomActor::New(); + Stage::GetCurrent().Add( derived ); + + // should have all actor properties + DALI_TEST_EQUALS( derived.GetPropertyType( Actor::Property::WORLD_MATRIX ), Property::MATRIX, TEST_LOCATION ); + auto actorHandle = Actor::New(); + DALI_TEST_EQUALS( derived.GetPropertyCount(), actorHandle.GetPropertyCount(), TEST_LOCATION ); + + // add a property in base class + AnimatablePropertyRegistration( typeRegistration, "Foobar", ANIMATABLE_PROPERTY_REGISTRATION_START_INDEX, 10.f ); + + // should be one more property now + DALI_TEST_EQUALS( derived.GetPropertyCount(), actorHandle.GetPropertyCount() + 1, TEST_LOCATION ); + // check that the default value is set for base class + DALI_TEST_EQUALS( UnregisteredCustomActor::New().GetProperty(ANIMATABLE_PROPERTY_REGISTRATION_START_INDEX).Get(), 10.f, TEST_LOCATION ); + // check that the default value is set for the derived instance as well + DALI_TEST_EQUALS( derived.GetProperty(ANIMATABLE_PROPERTY_REGISTRATION_START_INDEX).Get(), 10.f, TEST_LOCATION ); END_TEST; } diff --git a/dali/internal/event/common/object-impl.cpp b/dali/internal/event/common/object-impl.cpp index 3ea3729..7bf745d 100644 --- a/dali/internal/event/common/object-impl.cpp +++ b/dali/internal/event/common/object-impl.cpp @@ -1121,7 +1121,11 @@ void Object::RegisterAnimatableProperty( const TypeInfo& typeInfo, } else { - initialValue = typeInfo.GetPropertyDefaultValue( index ); + initialValue = typeInfo.GetPropertyDefaultValue( index ); // recurses type hierarchy + if( Property::NONE == initialValue.GetType() ) + { + initialValue = Property::Value( typeInfo.GetPropertyType( index ) ); // recurses type hierarchy + } } RegisterSceneGraphProperty( propertyName, Property::INVALID_KEY, index, initialValue ); AddUniformMapping( index, propertyName ); diff --git a/dali/internal/event/common/type-info-impl.cpp b/dali/internal/event/common/type-info-impl.cpp index 65f63fc..fe1fd20 100644 --- a/dali/internal/event/common/type-info-impl.cpp +++ b/dali/internal/event/common/type-info-impl.cpp @@ -941,10 +941,13 @@ Property::Value TypeInfo::GetPropertyDefaultValue( Property::Index index ) const { return iter->second; } - else + // we didn't have a value so ask base + if( GetBaseType( mBaseType, mTypeRegistry, mBaseTypeName ) ) { - return Property::Value( GetPropertyType( index ) ); + // call base type recursively + return mBaseType->GetPropertyDefaultValue( index ); } + return Property::Value(); // return none } void TypeInfo::SetProperty( BaseObject *object, Property::Index index, const Property::Value& value ) const -- 2.7.4