Skip to content

Commit c0dc88b

Browse files
committed
Report pending text reshapes in scene content-changed check and skip redundant holder dirtying.
1 parent e1e59a6 commit c0dc88b

10 files changed

Lines changed: 183 additions & 9 deletions

File tree

include/pagx/PAGComposition.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,11 @@ class PAGComposition : public PAGLayer {
190190
// ViewModel/Animation-driven text once per draw. fontConfig is borrowed for the call.
191191
void flushTextHolders(FontConfig* fontConfig);
192192

193+
// Returns true if any TextHolder in this composition's binding (or a descendant composition's)
194+
// has a pending reshape. Used by PAGScene::hasContentChanged so a dirty holder is not dropped by
195+
// the dirty gate before flush runs.
196+
bool hasDirtyTextHolders() const;
197+
193198
friend class PAGScene;
194199
};
195200

playground/pagx-viewer/src/cpp/PAGXView.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <emscripten/html5.h>
2222
#include <algorithm>
2323
#include <cstdint>
24+
#include "pagx/PAGAnimation.h"
2425
#include "pagx/PAGXImporter.h"
2526
#include "pagx/tgfx.h"
2627
#include "pagx/types/Data.h"

resources/text_reshape_demo.pagx

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<pagx width="400" height="200" viewModel="@MainVM">
3+
<Resources>
4+
<ViewModel id="MainVM">
5+
<Property id="title" name="title" type="String" default="VM Default"/>
6+
<Property id="size" name="size" type="Number" default="40"/>
7+
</ViewModel>
8+
</Resources>
9+
10+
<Layer id="animLayer" name="AnimText" width="400" height="100">
11+
<Group>
12+
<Text id="animText" text="Hello" fontFamily="NotoSansSC" fontSize="40" position="20,70"/>
13+
<Fill color="#000000"/>
14+
</Group>
15+
<Timelines>
16+
<Animation ref="@textAnim"/>
17+
</Timelines>
18+
</Layer>
19+
20+
<Layer id="vmLayer" name="VMText" width="400" height="100" y="100">
21+
<Group>
22+
<Text id="vmText" text="VM Default" fontFamily="NotoSansSC" fontSize="40" position="20,170"/>
23+
<Fill color="#0000FF"/>
24+
</Group>
25+
</Layer>
26+
27+
<DataBind source="$vm.title" target="@vmText" channel="text"/>
28+
<DataBind source="$vm.size" target="@vmText" channel="fontSize"/>
29+
30+
<Animations>
31+
<Animation id="textAnim" duration="120" frameRate="60" loop="once">
32+
<Object target="animText">
33+
<Channel name="text" type="string">
34+
<Key time="0" value="Hello" interpolation="hold"/>
35+
<Key time="60" value="Animated" interpolation="hold"/>
36+
</Channel>
37+
</Object>
38+
</Animation>
39+
</Animations>
40+
</pagx>

src/pagx/PAGScene.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -764,7 +764,16 @@ tgfx::DisplayList* PAGScene::getDisplayListForOptions() const {
764764
}
765765

766766
bool PAGScene::hasContentChanged() const {
767-
return displayList != nullptr && displayList->hasContentChanged();
767+
if (displayList == nullptr) {
768+
return false;
769+
}
770+
// A pending TextHolder reshape (ViewModel/Animation drove a text-shaping channel) has not yet
771+
// touched the tgfx objects — flush runs inside Record(). Without this check the dirty gate would
772+
// skip the frame and the reshape would never reach the screen.
773+
if (_rootComposition != nullptr && _rootComposition->hasDirtyTextHolders()) {
774+
return true;
775+
}
776+
return displayList->hasContentChanged();
768777
}
769778

770779
std::unique_ptr<tgfx::Recording> Record(tgfx::Context* context,

src/pagx/runtime/PAGComposition.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,4 +483,18 @@ void PAGComposition::flushTextHolders(FontConfig* fontConfig) {
483483
}
484484
}
485485

486+
bool PAGComposition::hasDirtyTextHolders() const {
487+
if (binding != nullptr && binding->hasDirtyTextHolders()) {
488+
return true;
489+
}
490+
std::vector<PAGComposition*> childComps = {};
491+
CollectChildCompositions(const_cast<PAGComposition*>(this), childComps);
492+
for (auto* childComp : childComps) {
493+
if (childComp->hasDirtyTextHolders()) {
494+
return true;
495+
}
496+
}
497+
return false;
498+
}
499+
486500
} // namespace pagx

src/renderer/LayerBuilder.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,15 @@ void RuntimeBinding::flushTextHolders(FontConfig* fontConfig) {
131131
}
132132
}
133133

134+
bool RuntimeBinding::hasDirtyTextHolders() const {
135+
for (const auto& holder : textHolders) {
136+
if (holder != nullptr && holder->isDirty()) {
137+
return true;
138+
}
139+
}
140+
return false;
141+
}
142+
134143
void RuntimeBinding::remove(const Node* node) {
135144
targets.erase(node);
136145
std::vector<std::shared_ptr<TextHolder>> stillAlive;

src/renderer/LayerBuilder.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,10 @@ struct RuntimeBinding {
318318
// Animation channels have been applied. fontConfig is borrowed for the duration of the call.
319319
void flushTextHolders(FontConfig* fontConfig);
320320

321+
// Returns true if any TextHolder has a pending reshape (apply recorded a change since the last
322+
// flush). Used by PAGScene's content-changed check so a dirty holder is not skipped before flush.
323+
bool hasDirtyTextHolders() const;
324+
321325
private:
322326
// Returns the existing target for the node, creating a plain RuntimeTarget if none exists yet.
323327
RuntimeTarget* ensureTarget(const Node* node) {

src/renderer/TextHolder.cpp

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,46 +66,56 @@ void TextHolder::apply(Text* node, const std::string& channel, const KeyValue& v
6666
auto& glyph = entry->glyph;
6767
// Continuous attributes mix from the current value (the role a tgfx object plays for other
6868
// channels); discrete attributes ignore mix and overwrite, matching the existing writer
69-
// convention (WriteLayerBlendMode / WriteLayerName).
69+
// convention (WriteLayerBlendMode / WriteLayerName). Only mark dirty when the value actually
70+
// changes: Hold-interpolated animation channels re-apply the same value every advance, so a
71+
// blind dirty flag would trigger a full reshape at every draw.
7072
if (channel == "fontSize") {
7173
const auto* v = std::get_if<float>(&value);
7274
if (v == nullptr) {
7375
return;
7476
}
75-
glyph.fontSize = MixFloat(glyph.fontSize, *v, mix);
77+
auto mixed = MixFloat(glyph.fontSize, *v, mix);
78+
if (mixed == glyph.fontSize) {
79+
return;
80+
}
81+
glyph.fontSize = mixed;
7682
} else if (channel == "letterSpacing") {
7783
const auto* v = std::get_if<float>(&value);
7884
if (v == nullptr) {
7985
return;
8086
}
81-
glyph.letterSpacing = MixFloat(glyph.letterSpacing, *v, mix);
87+
auto mixed = MixFloat(glyph.letterSpacing, *v, mix);
88+
if (mixed == glyph.letterSpacing) {
89+
return;
90+
}
91+
glyph.letterSpacing = mixed;
8292
} else if (channel == "text") {
8393
const auto* v = std::get_if<std::string>(&value);
84-
if (v == nullptr) {
94+
if (v == nullptr || *v == glyph.text) {
8595
return;
8696
}
8797
glyph.text = *v;
8898
} else if (channel == "fontFamily") {
8999
const auto* v = std::get_if<std::string>(&value);
90-
if (v == nullptr) {
100+
if (v == nullptr || *v == glyph.fontFamily) {
91101
return;
92102
}
93103
glyph.fontFamily = *v;
94104
} else if (channel == "fontStyle") {
95105
const auto* v = std::get_if<std::string>(&value);
96-
if (v == nullptr) {
106+
if (v == nullptr || *v == glyph.fontStyle) {
97107
return;
98108
}
99109
glyph.fontStyle = *v;
100110
} else if (channel == "fauxBold") {
101111
const auto* v = std::get_if<bool>(&value);
102-
if (v == nullptr) {
112+
if (v == nullptr || *v == glyph.fauxBold) {
103113
return;
104114
}
105115
glyph.fauxBold = *v;
106116
} else if (channel == "fauxItalic") {
107117
const auto* v = std::get_if<bool>(&value);
108-
if (v == nullptr) {
118+
if (v == nullptr || *v == glyph.fauxItalic) {
109119
return;
110120
}
111121
glyph.fauxItalic = *v;

src/renderer/TextHolder.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,13 @@ class TextHolder {
7777
// borrowed for the duration of the call to build a LayoutContext; the holder does not retain it.
7878
void flush(FontConfig* fontConfig);
7979

80+
// Returns true if apply() has recorded a change since the last flush(). Callers (PAGScene's
81+
// content-changed check) use this to avoid skipping a frame that would otherwise drop the reshape
82+
// before flush runs.
83+
bool isDirty() const {
84+
return dirty;
85+
}
86+
8087
// Drops the entry for the given node (called when its runtime target is being removed from the
8188
// binding, so a later flush never dereferences a freed target). Returns true if the holder has
8289
// no entries left, letting the binding drop the now-empty holder.

test/src/PAGXTest.cpp

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13566,5 +13566,80 @@ PAGX_TEST(PAGXTest, SMDataBindTypeMismatch) {
1356613566
EXPECT_EQ(smTimeline->getCurrentState("main"), "low");
1356713567
}
1356813568

13569+
// Smoke test: the demo pagx file (animation + viewmodel driving text channels) parses cleanly.
13570+
PAGX_TEST(PAGXTest, TextReshapeDemoParses) {
13571+
auto doc =
13572+
pagx::PAGXImporter::FromFile(ProjectPath::Absolute("resources/text_reshape_demo.pagx"));
13573+
ASSERT_NE(doc, nullptr);
13574+
for (const auto& err : doc->errors) {
13575+
FAIL() << "parse error: " << err;
13576+
}
13577+
EXPECT_EQ(doc->layers.size(), 2u);
13578+
EXPECT_EQ(doc->animations.size(), 1u);
13579+
EXPECT_EQ(doc->dataBinds.size(), 2u);
13580+
ASSERT_NE(doc->viewModel, nullptr);
13581+
EXPECT_EQ(doc->viewModel->id, "MainVM");
13582+
EXPECT_NE(doc->findNode<pagx::Animation>("textAnim"), nullptr);
13583+
auto* vmText = doc->findNode<pagx::Text>("vmText");
13584+
ASSERT_NE(vmText, nullptr);
13585+
EXPECT_EQ(vmText->text, "VM Default");
13586+
}
13587+
13588+
// Verifies the demo pagx's animation and viewmodel both reshape text through the runtime holder:
13589+
// after advancing the default timeline past the second keyframe, the animated Text's blob width
13590+
// changes to the "Animated" glyph extent; after driving the ViewModel's "title" property, the
13591+
// bound Text's blob width changes to the new content. Both go through PAGScene::draw so the
13592+
// holder's flush (setTextBlob) reflects in the bound tgfx::Text.
13593+
PAGX_TEST(PAGXTest, TextReshapeDemoAnimationAndViewModel) {
13594+
auto doc =
13595+
pagx::PAGXImporter::FromFile(ProjectPath::Absolute("resources/text_reshape_demo.pagx"));
13596+
ASSERT_NE(doc, nullptr);
13597+
pagx::FontConfig fontConfig;
13598+
for (const auto& fontPath : GetFallbackFontPaths()) {
13599+
fontConfig.addFallbackFont(fontPath, 0);
13600+
}
13601+
doc->applyLayout(&fontConfig);
13602+
auto scene = pagx::PAGScene::Make(doc);
13603+
ASSERT_NE(scene, nullptr);
13604+
auto surface = pagx::PAGSurface::MakeOffscreen(400, 200);
13605+
ASSERT_NE(surface, nullptr);
13606+
13607+
auto* animText = doc->findNode<pagx::Text>("animText");
13608+
ASSERT_NE(animText, nullptr);
13609+
auto* vmTextNode = doc->findNode<pagx::Text>("vmText");
13610+
ASSERT_NE(vmTextNode, nullptr);
13611+
13612+
// First frame: animation at time 0 ("Hello"), viewmodel default ("VM Default").
13613+
EXPECT_TRUE(scene->draw(surface));
13614+
auto binding = scene->mutableBinding();
13615+
ASSERT_NE(binding, nullptr);
13616+
auto animRuntime = binding->get<tgfx::Text>(animText);
13617+
ASSERT_NE(animRuntime, nullptr);
13618+
ASSERT_NE(animRuntime->textBlob(), nullptr);
13619+
float animWidthBefore = animRuntime->textBlob()->getBounds().width();
13620+
auto vmRuntime = binding->get<tgfx::Text>(vmTextNode);
13621+
ASSERT_NE(vmRuntime, nullptr);
13622+
ASSERT_NE(vmRuntime->textBlob(), nullptr);
13623+
float vmWidthBefore = vmRuntime->textBlob()->getBounds().width();
13624+
13625+
// Advance the document-level animation past frame 60 (1s at 60fps) so the Hold keyframe value
13626+
// "Animated" takes effect, then draw.
13627+
auto defaultTimeline = scene->getDefaultTimeline();
13628+
ASSERT_NE(defaultTimeline, nullptr);
13629+
defaultTimeline->advanceAndApply(1'000'000);
13630+
EXPECT_TRUE(scene->draw(surface));
13631+
ASSERT_NE(animRuntime->textBlob(), nullptr);
13632+
float animWidthAfter = animRuntime->textBlob()->getBounds().width();
13633+
// "Animated" is longer than "Hello", so the blob width must grow.
13634+
EXPECT_GT(animWidthAfter, animWidthBefore) << "animation did not reshape animText to 'Animated'";
13635+
13636+
// Drive the ViewModel's "title" property and draw: the bound vmText must reshape.
13637+
scene->viewModel()->propertyString("title")->value("ReshapedByVM");
13638+
EXPECT_TRUE(scene->draw(surface));
13639+
ASSERT_NE(vmRuntime->textBlob(), nullptr);
13640+
float vmWidthAfter = vmRuntime->textBlob()->getBounds().width();
13641+
EXPECT_NE(vmWidthAfter, vmWidthBefore) << "viewmodel did not reshape vmText to 'ReshapedByVM'";
13642+
}
13643+
1356913644
// Canonical scene render test: Composition-wrapped layer with animation via scene.
1357013645
} // namespace pag

0 commit comments

Comments
 (0)