BR: setComponentEffect on JUCE 8 (overeager image caching?)

Hi @reuk!

Got a fun one. This took me a couple hours to track down today and yesterday. I busted out tsan, asan, and was half convinced I was going nuts.

Issue: A lot of glitchy painting on a visualizer that only happens only the juce8 branch. It turned out that the problem was setComponentEffect (on juce8).

Reproduction: I have 10+ components — one for each voice in my synth — each of which call setComponentEffect.

Because my visualizer is driven by audio, the 10 components each start off “empty” in some black default state. On JUCE 7, when a synth voice is played, one of the components fills up with lights.

I’m not fully intimate with the changes in juce8, but from taking a glance maybe we’ve got some overeager cache hashing going on? When I play a note on juce8, every single component appears to get a “copy” of the most recent active voice. In other words, all 10 components seem to be pointing to the same image cache (this also causes somewhat crazy flickering).

If you like I can cook up a minimal example. But it sort of seems like there needs to be an additional piece of data going into the image hashing key (maybe something component-specific like an internal component id?) to allow multiple cached images with the same dimensions.

(Love the idea of caching the image effect, but also wondering how the cache can be broken, as for example, my image effect is essentially animated!)

Thanks, I think I can repro something similar. At the moment I’m only seeing broken caching on macOS. Please could you let me know on which platforms you’re seeing problems? If you’re also seeing issues on Windows, then I might have found a different bug.

Just in case, my test looks like this:

class Cached : public juce::Component
{
public:
    Cached()
    {
        juce::Random random;
        phase = random.nextFloat();
        increment = random.nextFloat() * 0.01f;

        effect.setShadowProperties (juce::DropShadow { juce::Colours::black, 10, {} });
        setComponentEffect (&effect);
    }

    void paint (juce::Graphics& g) override
    {
        g.setColour (juce::Colours::cyan);
        const auto bounds = getLocalBounds()
                                .toFloat()
                                .removeFromBottom ((float) getHeight() * phase)
                                .reduced (5.0f);
        g.drawRoundedRectangle (bounds, 10.0f, 3.0f);
    }

private:
    juce::DropShadowEffect effect{};
    juce::VBlankAttachment attachment { this, [this]
    {
        phase += increment;
        phase -= (float) (int) phase;
        repaint();
    } };
    float phase{}, increment{};
};

class MainComponent final : public juce::Component
{
public:
    MainComponent()
    {
        setSize (600, 400);

        for (auto& c : cached)
            addAndMakeVisible (c);
    }

    void paint (juce::Graphics& g) override
    {
        g.fillAll (juce::Colours::white);
    }

    void resized() override
    {
        juce::FlexBox fb;

        for (auto& c : cached)
            fb.items.add (juce::FlexItem { c }.withWidth (30));

        fb.performLayout (getLocalBounds());
    }

private:
    std::vector<Cached> cached = std::vector<Cached> (10);
};
1 Like

Hey — yes, macOS. Nice Repro!

I’ve thought about this a bit more and I think it’s probably not ideal to add that effect image caching in Component::paintEntireComponent to begin with — Not 100% sure, but it seems it was put in place because of the need for faster drop shadow rendering (ironic, since it broke my fast animated drop shadows!). I’m thinking it should be something that DropShadowEffect handles internally?.. Or maybe ImageEffectFilter can gain some caching super powers (and derived classes can add to the cache hash key or when to break cache)?..

The reasoning: caching at the current level (without providing an API to break the cache) eliminates whole classes of effects. For example, I have a reflection effect: as the source component changes, the reflection changes. I also have my own drop shadow effect that internally caches until the content changes, at which point the cache changes.

Basically this cache causes breakage for non-shadow component effects where the image content changes after first render, anything with animation, etc.

I think the current caching is mainly trying to avoid allocating a fresh image each time the component is painted. At the moment I’m planning to have each Component that is using an effect store a ‘scratch’ Image internally that it can use for rendering, avoiding ImageCache completely. Hopefully this approach should restore the old rendering behaviour without slowing things down too much on Windows.

1 Like

Ah right, I see. It’s mainly about retrieving an existing allocated image to paint onto. Sorry, panicked a bit there, should have read more of the code! :smiley:

If you wanted to stick with the ImageCache route, you could add a public/protected juce::Uuid to Component to use as the key. I think right now there’s no unique identifier for a component (and I keep running into good use cases for one).

Thanks for your patience. I believe this commit fixes the issue:

Please give it a go and let us know if you run into further problems.

1 Like

Haven’t tested it extensively, but it seems all fixed up on first inspection — thank you!