rive-app / rive-app/rive-runtime

`Image::controlSize()` skip-if-unchanged optimization prevents `updateImageScale()` from running after dynamic image load

Open Beginner friendly
#97 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
1.2k
Forks
121
PR merge metrics
No merged PRs in 30d

Description

Summary

When a dynamic image is injected via DataBinding into an Image component inside a Layout, the controlSize() method skips calling updateImageScale() because the layout dimensions haven't changed since the previous call. However, during the previous call, the renderImage was NULL (the image hadn't been decoded yet), so the Fit/Scale math was skipped. When the image finally decodes and renderImage becomes valid, controlSize() is called again with the same dimensions, but the skip optimization prevents updateImageScale() from ever running with both a valid renderImage AND valid layout dimensions.

Severity

High — Affects all dynamic image injection workflows using DataBinding with Layout containers. Images render with the editor-baked scale values instead of the computed Fit scale.

Environment

  • Runtime: rive-cpp (C++ runtime)
  • Platform: Windows (D3D11 renderer)
  • Rive File: Layout containing an Image with DataBinding ViewModel image property

Steps to Reproduce

  1. Create a Rive file with an Image inside a Layout, with Fit set to Contain
  2. Bind the Image to a ViewModel Image property
  3. Load the .riv file and inject a dynamic image at runtime
  4. Observe that the image renders with incorrect scale (using the editor-baked scaleX/scaleY values instead of the computed Fit scale)

Root Cause Analysis

The renderImage pointer and m_layoutWidth/m_layoutHeight become valid at different times during dynamic image injection:

Timeline
T1: File loads
    → controlSize(547, 461)         ← m_layoutWidth = 547, m_layoutHeight = 461
    → updateImageScale()            ← renderImage == NULL → scale math SKIPPED

T2: Dynamic image injected via DataBinding
    → Image::setAsset()
      → markParentLayoutDirty()
      → updateImageScale()          ← renderImage == NULL (not decoded yet)
                                       m_layoutWidth = NaN (clone starts fresh)
                                       → scale math SKIPPED

T3: Layout propagation fires
    → controlSize(547, 461)         ← m_layoutWidth was NaN → 547 ≠ NaN → enters block
    → m_layoutWidth = 547
    → updateImageScale()            ← renderImage STILL NULL → scale math SKIPPED

T4: Image decodes, renderImage becomes valid
    → assetUpdated()
    → markParentLayoutDirty()
    → updateImageScale()            ← renderImage VALID, BUT m_layoutWidth = NaN
                                       (on cloned artboard instance) → SKIPPED

T5: Layout propagation fires again
    → controlSize(547, 461)         ← 547 == 547 → UNCHANGED → SKIPPED entirely!
                                       updateImageScale() NEVER CALLED with both valid

The renderImage and layout dimensions are like "ships passing in the night" — they're never both valid at the same time when updateImageScale() runs.

Affected Code

File: src/shapes/image.cpp
Function: Image::controlSize()

void Image::controlSize(Vec2D size, ...) {
    if (m_layoutWidth != size.x || m_layoutHeight != size.y) {  // ← skip guard
        m_layoutWidth = size.x;
        m_layoutHeight = size.y;
        updateImageScale();  // ← never called when renderImage is valid
    }
}

And in updateImageScale():

void Image::updateImageScale() {
    auto renderImage = imageAsset()->renderImage();
    if (renderImage != nullptr && !std::isnan(m_layoutWidth) && !std::isnan(m_layoutHeight)) {
        // Scale math — only runs when BOTH renderImage AND layout are valid
        // But due to the skip guard above, this combination never occurs
    }
}

Proposed Fix

Remove the skip-if-unchanged optimization. Always update dimensions and call updateImageScale():

void Image::controlSize(Vec2D size,
                        LayoutScaleType widthScaleType,
                        LayoutScaleType heightScaleType,
                        LayoutDirection direction)
{
    m_layoutWidth = size.x;
    m_layoutHeight = size.y;
    updateImageScale();
}

The updateImageScale() function already has internal guards (renderImage != nullptr, !isnan) that prevent unnecessary work. The outer skip guard is redundant and creates the race condition.

If performance is a concern, a more targeted fix: always call updateImageScale() but keep the dimension update conditional:

m_layoutWidth = size.x;
m_layoutHeight = size.y;
updateImageScale();  // Always recalculate — renderImage may have changed

Diagnostic Evidence

Before fix — renderImage is valid but layoutW/H are NaN, then controlSize skips because values unchanged:

[RIVE DIAG] markParentLayoutDirty: FOUND parent layout!
[RIVE DIAG] updateImageScale: renderImage=000001FEF7B49DC0, layoutW=nan, layoutH=nan, isNanW=1, isNanH=1
...
[RIVE DIAG] Image::controlSize called: w=547.000000, h=461.000000
(no updateImageScale log follows — skipped because 547 == 547)

After fix — controlSize always calls updateImageScale, which now runs with both valid renderImage and valid layout:

[RIVE DIAG] markParentLayoutDirty: FOUND parent layout!
[RIVE DIAG] updateImageScale: renderImage=000001A290F997C0, layoutW=547.000000, layoutH=461.000000, isNanW=0, isNanH=0
[RIVE DIAG] updateImageScale: COMPUTING fit=1, imgW=2160.000000, imgH=3840.000000, layoutW=547.000000, layoutH=461.000000
[RIVE DIAG] updateImageScale: CONTAIN s=0.120052, newScaleX=0.120052, newScaleY=0.120052
...
[RIVE DIAG] Image::controlSize called: w=547.000000, h=461.000000 (prev w=547.000000, h=461.000000)
[RIVE DIAG] updateImageScale: renderImage=000001A290F997C0, layoutW=547.000000, layoutH=461.000000, isNanW=0, isNanH=0
[RIVE DIAG] updateImageScale: COMPUTING fit=1, imgW=2160.000000, imgH=3840.000000, layoutW=547.000000, layoutH=461.000000
[RIVE DIAG] updateImageScale: CONTAIN s=0.120052, newScaleX=0.120052, newScaleY=0.120052

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/shapes/image.cpp by reading Image::controlSize() and updateImageScale(), then follow the dynamic image injection sequence through setAsset() and assetUpdated(). Reproduce the Layout/DataBinding case described in the issue. Done when a decoded image recomputes its Fit/Scale values even when the layout dimensions are unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
computer-graphics
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.