HISE · ScriptDynamicContainer
Dynamic Container Fixes
Thirteen fixes found while moving the controls of a project's interface onto Content.addDynamicContainer(). Three of the bugs crash HISE and one freezes it, all in the way a script normally uses the container: build the children with setData(), then set their values and callbacks.
Branch develop_FixDynamicContainer
Base develop 3fe57acb6 (CSS fixes pt. 5)
Commits 13
Files 12, +499 / −80
Summary
Commit 2 changes valuetree::AnyPropertyListener and valuetree::PropertyListener, which other parts of HISE use too. It is the one change outside the container that needs a careful look.
The fixes
1
A click on a disabled control sends the keyboard focus round in circles
crashd7e145b42
- Symptom
- Clicking a disabled dynamic Button overflows the stack. The same happens with a disabled static component that wants the keyboard focus, for example one with a key press callback.
- Cause
SimpleTraverser::getDefaultComponent() returned the parent itself whenever it wanted the focus, even when disabled. Component::grabKeyboardFocusInternal() asks a component that cannot take the focus for its default component and hands the focus on to it, which recurses.- Fix
- Return the parent only when it is enabled. The container's Button and ComboBox no longer want the focus, like
HiToggleButton and HiComboBox.
- Files
- ScriptingContentComponent.cpp, DynamicComponentContainerTypes.cpp
2
ValueTree listeners skip only a repeated change
lost calls9777f0b8b
- Symptom
-
AnyPropertyListener: two properties set to the same value one after the other, and the second is never reported. The container's setValueCallback() loses a child's value this way.PropertyListener: the first change of a property to 0 is never reported.
- Cause
- The check meant for the priorised listener (called twice for one change) compared only the value, from any property.
PropertyListener compared against lastValues[id] for an id it had not seen, an empty var, which equals 0.
- Fix
- Skip only the same property with the same value, and only a value that was seen before.
- Files
- ValueTreeHelpers.cpp, ValueTreeHelpers.h
3
A lock for the container's value trees
crash5a72d1f61
- Symptom
setData() followed by getComponent(), setControlCallback() or setValue() in the same callback crashes HISE: heap corruption in dyncomp::Base::~Base and in ~ChildReference. In a loop of rebuilds it crashed within 10.- Cause
setData() builds and destroys the components later on the message thread (WrapperComponent::onChange, Base::updateChild, ~Base, Data::onValueChange). The script keeps changing the same juce::ValueTrees on the scripting thread. A ValueTree is not thread safe, and the message thread side never took the script lock.- Fix
-
dyncomp::Data::getLock() is held for every access to the data and value trees, from both threads.- The
valuetree:: listeners hold their own lock while they call back, and the scripting thread waits for it while holding the data lock. So a component handler must not wait for the data lock inside a listener callback (that deadlocked in testing). Base::deferUpdate() queues the handlers to run right after on the message thread, in order, guarded by a SafePointer.
- The
changed and resetValueToDefault refreshes run after the broadcast, since the broadcaster holds its own lock while it calls back.
The rules are in the comment on getLock(): hold it briefly, never while waiting for another lock or running script code, and never inside a refreshBroadcaster callback.
- Files
- DynamicComponentContainer.cpp/.h, DynamicComponentContainerTypes.cpp, ScriptComponentWrappers.h, ScriptingApiContent.cpp/.h
4
Paint routines run without the refresh broadcaster
freeze261361593
- Symptom
- Remove a child with
removeFromParent() or removeAllChildren(), keep a reference to it, then call sendRepaintMessage() or changed() on any child: the message thread spins forever. The interface and every script timer stop.
- Cause
- Every
ChildReference listened to Data::refreshBroadcaster to run its paint routine. During the broadcast, isValid() found its child gone and called refreshBroadcaster.removeListener(): the write lock waits for the read lock the broadcast is holding.
- Fix
- References no longer listen.
sendMessage(repaint) queues the paint routines of the references the message reaches (repaintChildReferences(), repaintIfReachedBy()). This also stops sendRepaintMessage() on one child from running every paint routine in the container.
- Files
- ScriptingApiContent.cpp/.h
5
Control callbacks fire on every change and never on setValue()
lost calls5d68735ef
- Symptom
-
- A button switched on by
setValue() and then clicked off by hand is never heard: the button shows off, the DSP stays on.
setControlCallback() fires the callback once right away, except when the value is 0.- After
setData(), the container's value callback reports every old child as undefined.
- Cause
ChildReference::onValue() skipped a value equal to the last one it sent, and before the first call an empty var counted as 0. setValue() used setPropertyExcludingListener(), so the listener's own last value went stale.- Fix
setValue() lets the listener see the change and only mutes the callback (muteValueCallback). Every real change reaches the callback. Registering a callback does not fire it. setData() shuts the old value callback down.- Files
- ScriptingApiContent.cpp/.h
6
removeAllChildren() keeps a child added right after it
lost child45c886d5b
- Symptom
removeAllChildren() followed by addChildComponent() loses the new child.- Cause
- The removal runs later and removed whatever children there were by then.
- Fix
- It removes the children there were at the call.
- Files
- ScriptingApiContent.cpp
7
Slider fixes
wrongc9f0c6ac3
- Fix
-
style: "Vertical" gave LinearHorizontal. It is LinearBarVertical now.defaultValue was only read when the slider was built. A change now updates the double-click value.- The slider is built again when its processor connection is set up (right after the constructor). An unconnected one then showed its minimum until the value changed. It takes the current value now.
- A linear bar without a text box got JUCE's 1px
textBoxOutlineColourId outline. ScriptSlider clears it too.
- Files
- DynamicComponentContainerTypes.cpp
8
ComboBox shows the item of a Category::Item list
lookb7951ef4f
- Symptom
- With
items like CLIP::HARD, the list's text is the whole CLIP::HARD. A static ComboBox shows HARD.
- Cause
setSelectedId() ran before rebuildPopupMenu(), which is what splits the items into submenus.- Fix
- Rebuild first.
- Files
- DynamicComponentContainerTypes.cpp
9
FloatingTile children
crashf8ac5aa98
- Symptom
-
- Any FloatingTile child crashes HISE.
- A FloatingTile child without data crashes it as well.
- A
MatrixPeakMeter tile looks different from the same static tile.
- Cause
setData() wrapped { ContentProperties, FloatingTileData } into an array, so the data never found its FloatingTileData, and getFloatingTileData() dereferenced the null object. Without data, forwardToFirstChild() still returned true with no child. The container forced its own look and feel onto the tile's content.- Fix
- The object form goes in as it is, the null data is checked, and a tile without data does not forward. The tile is set up like the wrapper of a
ScriptFloatingTile (setIsFloatingTileOnInterface(), not opaque, setContent(), refreshRootLayout()). Only a scripted look and feel replaces the content's own.
- Files
- DynamicComponentContainer.h, DynamicComponentContainerTypes.cpp, ScriptingApiContent.cpp
10
Panel draws a paint routine set after it was built
lookc00fcca9f
- Symptom
setPaintRoutine() after setData(), once the interface is shown, draws nothing. The panel stays a plain box.- Cause
- The Panel looked up its draw handler only when it was built.
- Fix
- A repaint message hands the panel the current handler, through the new
BorderPanel::setDrawHandler(). setPaintRoutine() sends that message.
- Files
- DynamicComponentContainerTypes.cpp, MiscComponents.cpp/.h
11
Clicks on the empty area go through to the components below
blocksc3e098636
- Symptom
- A container laid over a panel blocks the panel's own mouse callback and every static component under it, even where the container has no child.
- Cause
- The wrapper and the root component took every click inside the container's bounds. Setting the wrapper to let clicks through is not enough:
ScriptContentComponent::updateComponentVisibility() sets every wrapper's click interception from ScriptComponent::isClickable().
- Fix
- The container returns false from
isClickable(), and the root lets clicks through to its children only. A broadcaster on the container still hears every child, since it listens to the nested components.
- Files
- ScriptingApiContent.h, DynamicComponentContainerTypes.cpp
12
No context menu where every item is disabled
wrong4e3470d7c
- Symptom
attachToContextMenu() on a container opens the menu on every child, greyed out where it does not apply. On a slider pack it also takes the right click that draws a line.- Fix
- On a container, a menu with no enabled item is not shown. The state function decides per child by disabling every item. Other components keep the current behaviour.
- Files
- ScriptComponentWrappers.cpp
13
Not saved in user presets by default
tidy899b80848
- Fix
- The container has no value of its own, so
saveInPreset defaults to false. Its children are stored with addStateToUserPreset().
- Files
- ScriptingApiContent.cpp
How it was tested
- A test script built the container and set values straight after
setData(), with script writes and repaints running at the same time: 230 and then 142 rebuilds, no crash and no hang. Before the fixes it crashed within 10.
- Removing children, keeping a reference to a removed one, then 200 repaints: no freeze. Five recompiles that call the container straight away: no hang.
- Every child type from a script: knob, bar, button (toggle, momentary, radio), ComboBox with submenus, SliderPack on registered data, Panel with a paint routine, a nested Panel, FloatingTile with and without data.
- In a real project, six panels build their controls in one container each, from 38 different layouts. Every layout builds without errors, edits on every control type reach the DSP, 60 fast rebuilds run clean, and the interface was tested by hand.
Found but not changed
- A container Slider has no
mouseSensitivity or dragDirection, and an unknown property in set() is a script error. Its step size is clamped to 0..1 by RangeHelpers.
ContainerChild.sendRepaintMessage() takes one argument, where a ScriptComponent's takes none. Easy to trip over when moving code to a container.- REST API:
ui/apply cannot add a ScriptDynamicContainer ("Unknown component type"). /api/testing/screenshot paints a fresh interface before the container's queued updates run, so its children show the stock look.
- One script error in a look and feel function switches the whole look and feel off until the next compile. With a container, every child goes to the stock look at once.