Skip to content

Commit 1f1b1c5

Browse files
RepiteoRepiteo
committed
Merge pull request godotengine#104381 from Ivorforce/object-notification-nobool
Optimize `Object::notification` by avoiding runtime branches
2 parents 92002b1 + 8a76e31 commit 1f1b1c5

File tree

3 files changed

+91
-61
lines changed

3 files changed

+91
-61
lines changed

‎core/object/object.cpp‎

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -911,32 +911,44 @@ Variant Object::call_const(const StringName &p_method, const Variant **p_args, i
911911
return ret;
912912
}
913913

914-
void Object::notification(int p_notification, bool p_reversed) {
915-
if (p_reversed) {
916-
if (script_instance) {
917-
script_instance->notification(p_notification, p_reversed);
918-
}
919-
} else {
920-
_notificationv(p_notification, p_reversed);
921-
}
914+
void Object::_notification_forward(int p_notification) {
915+
// Notify classes starting with Object and ending with most derived subclass.
916+
// e.g. Object -> Node -> Node3D
917+
_notification_forwardv(p_notification);
922918

923919
if (_extension) {
924920
if (_extension->notification2) {
925-
_extension->notification2(_extension_instance, p_notification, static_cast<GDExtensionBool>(p_reversed));
921+
_extension->notification2(_extension_instance, p_notification, static_cast<GDExtensionBool>(false));
926922
#ifndef DISABLE_DEPRECATED
927923
} else if (_extension->notification) {
928924
_extension->notification(_extension_instance, p_notification);
929925
#endif // DISABLE_DEPRECATED
930926
}
931927
}
932928

933-
if (p_reversed) {
934-
_notificationv(p_notification, p_reversed);
935-
} else {
936-
if (script_instance) {
937-
script_instance->notification(p_notification, p_reversed);
929+
if (script_instance) {
930+
script_instance->notification(p_notification, false);
931+
}
932+
}
933+
934+
void Object::_notification_backward(int p_notification) {
935+
if (script_instance) {
936+
script_instance->notification(p_notification, true);
937+
}
938+
939+
if (_extension) {
940+
if (_extension->notification2) {
941+
_extension->notification2(_extension_instance, p_notification, static_cast<GDExtensionBool>(true));
942+
#ifndef DISABLE_DEPRECATED
943+
} else if (_extension->notification) {
944+
_extension->notification(_extension_instance, p_notification);
945+
#endif // DISABLE_DEPRECATED
938946
}
939947
}
948+
949+
// Notify classes starting with most derived subclass and ending in Object.
950+
// e.g. Node3D -> Node -> Object
951+
_notification_backwardv(p_notification);
940952
}
941953

942954
String Object::to_string() {

‎core/object/object.h‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -552,16 +552,17 @@ protected:
552552
_FORCE_INLINE_ void (Object::*_get_notification() const)(int) { \
553553
return (void(Object::*)(int)) & m_class::_notification; \
554554
} \
555-
virtual void _notificationv(int p_notification, bool p_reversed) override { \
556-
if (!p_reversed) { \
557-
m_inherits::_notificationv(p_notification, p_reversed); \
558-
} \
555+
virtual void _notification_forwardv(int p_notification) override { \
556+
m_inherits::_notification_forwardv(p_notification); \
559557
if (m_class::_get_notification() != m_inherits::_get_notification()) { \
560558
_notification(p_notification); \
561559
} \
562-
if (p_reversed) { \
563-
m_inherits::_notificationv(p_notification, p_reversed); \
560+
} \
561+
virtual void _notification_backwardv(int p_notification) override { \
562+
if (m_class::_get_notification() != m_inherits::_get_notification()) { \
563+
_notification(p_notification); \
564564
} \
565+
m_inherits::_notification_backwardv(p_notification); \
565566
} \
566567
\
567568
private:
@@ -705,7 +706,11 @@ class Object {
705706
virtual void _validate_propertyv(PropertyInfo &p_property) const {}
706707
virtual bool _property_can_revertv(const StringName &p_name) const { return false; }
707708
virtual bool _property_get_revertv(const StringName &p_name, Variant &r_property) const { return false; }
708-
virtual void _notificationv(int p_notification, bool p_reversed) {}
709+
710+
void _notification_forward(int p_notification);
711+
void _notification_backward(int p_notification);
712+
virtual void _notification_forwardv(int p_notification) {}
713+
virtual void _notification_backwardv(int p_notification) {}
709714

710715
static void _bind_methods();
711716
static void _bind_compatibility_methods() {}
@@ -889,7 +894,17 @@ class Object {
889894
return (cerr.error == Callable::CallError::CALL_OK) ? ret : Variant();
890895
}
891896

892-
void notification(int p_notification, bool p_reversed = false);
897+
// Depending on the boolean, we call either the virtual function _notification_backward or _notification_forward.
898+
// - Forward calls subclasses in descending order (e.g. Object -> Node -> Node3D -> extension -> script).
899+
// Backward calls subclasses in descending order (e.g. script -> extension -> Node3D -> Node -> Object).
900+
_FORCE_INLINE_ void notification(int p_notification, bool p_reversed = false) {
901+
if (p_reversed) {
902+
_notification_backward(p_notification);
903+
} else {
904+
_notification_forward(p_notification);
905+
}
906+
}
907+
893908
virtual String to_string();
894909

895910
// Used mainly by script, get and set all INCLUDING string.

‎tests/core/object/test_object.h‎

Lines changed: 42 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -464,72 +464,75 @@ TEST_CASE("[Object] Signals") {
464464
}
465465
}
466466

467-
class NotificationObject1 : public Object {
468-
GDCLASS(NotificationObject1, Object);
467+
class NotificationObjectSuperclass : public Object {
468+
GDCLASS(NotificationObjectSuperclass, Object);
469469

470470
protected:
471471
void _notification(int p_what) {
472-
switch (p_what) {
473-
case 12345: {
474-
order_internal1 = order_global++;
475-
} break;
476-
}
472+
order_superclass = ++order_global;
477473
}
478474

479475
public:
480-
static int order_global;
481-
int order_internal1 = -1;
482-
483-
void reset_order() {
484-
order_internal1 = -1;
485-
order_global = 1;
486-
}
476+
static inline int order_global = 0;
477+
int order_superclass = -1;
487478
};
488479

489-
int NotificationObject1::order_global = 1;
490-
491-
class NotificationObject2 : public NotificationObject1 {
492-
GDCLASS(NotificationObject2, NotificationObject1);
480+
class NotificationObjectSubclass : public NotificationObjectSuperclass {
481+
GDCLASS(NotificationObjectSubclass, NotificationObjectSuperclass);
493482

494483
protected:
495484
void _notification(int p_what) {
496-
switch (p_what) {
497-
case 12345: {
498-
order_internal2 = order_global++;
499-
} break;
500-
}
485+
order_subclass = ++order_global;
501486
}
502487

503488
public:
504-
int order_internal2 = -1;
505-
void reset_order() {
506-
NotificationObject1::reset_order();
507-
order_internal2 = -1;
489+
int order_subclass = -1;
490+
};
491+
492+
class NotificationScriptInstance : public _MockScriptInstance {
493+
void notification(int p_notification, bool p_reversed) override {
494+
order_script = ++NotificationObjectSuperclass::order_global;
508495
}
496+
497+
public:
498+
int order_script = -1;
509499
};
510500

511501
TEST_CASE("[Object] Notification order") { // GH-52325
512-
NotificationObject2 *test_notification_object = memnew(NotificationObject2);
502+
NotificationObjectSubclass *object = memnew(NotificationObjectSubclass);
513503

514-
SUBCASE("regular order") {
515-
test_notification_object->notification(12345, false);
504+
NotificationScriptInstance *script = memnew(NotificationScriptInstance);
505+
object->set_script_instance(script);
516506

517-
CHECK_EQ(test_notification_object->order_internal1, 1);
518-
CHECK_EQ(test_notification_object->order_internal2, 2);
507+
SUBCASE("regular order") {
508+
NotificationObjectSubclass::order_global = 0;
509+
object->order_superclass = -1;
510+
object->order_subclass = -1;
511+
script->order_script = -1;
512+
object->notification(12345, false);
519513

520-
test_notification_object->reset_order();
514+
CHECK_EQ(object->order_superclass, 1);
515+
CHECK_EQ(object->order_subclass, 2);
516+
// TODO If an extension is attached, it should come here.
517+
CHECK_EQ(script->order_script, 3);
518+
CHECK_EQ(NotificationObjectSubclass::order_global, 3);
521519
}
522520

523521
SUBCASE("reverse order") {
524-
test_notification_object->notification(12345, true);
525-
526-
CHECK_EQ(test_notification_object->order_internal1, 2);
527-
CHECK_EQ(test_notification_object->order_internal2, 1);
522+
NotificationObjectSubclass::order_global = 0;
523+
object->order_superclass = -1;
524+
object->order_subclass = -1;
525+
script->order_script = -1;
526+
object->notification(12345, true);
528527

529-
test_notification_object->reset_order();
528+
CHECK_EQ(script->order_script, 1);
529+
// TODO If an extension is attached, it should come here.
530+
CHECK_EQ(object->order_subclass, 2);
531+
CHECK_EQ(object->order_superclass, 3);
532+
CHECK_EQ(NotificationObjectSubclass::order_global, 3);
530533
}
531534

532-
memdelete(test_notification_object);
535+
memdelete(object);
533536
}
534537

535538
TEST_CASE("[Object] Destruction at the end of the call chain is safe") {

0 commit comments

Comments
 (0)