DropShadower impacting performance - recent JUCE change?

General JUCE discussion
#1

Has there been a recent change in or underlying DropShadower?

I noticed in a debug build that the responsiveness of my UI when updates were happening reasonably frequently was very poor. I was pressing a button and waiting several seconds for the button listener to be called. I suspected and investigated the message queue but there were no recent changes in my code that impacted the message queue traffic. And checking - my release builds seem to be fully responsive.

On profiling the offending code I found that about 50% of the CPU was being consumed inside juce_DropShadower.cpp - timerCallback, and within this ::isWindowOnCurrentVirtualDesktop.

I modified my main app window code to call ::setDropShadowEnabled(false) - and that huge CPU load has now gone and the UI is now responsive again in debug builds.

My curiosity is - this was not happening until recently. I only updated to JUCE 6.1.6 from 6.1.2 a few weeks ago and I suspect the change occurred then. I have never had responsiveness issues in a debug build before.

Is this a known issue?

#2

It looks like there were some changes to the implementation between those versions, mainly this this bug fix.

If you downgrade to 6.1.2, what does the usage drop to?

My experience with the current stock blur/shadows in JUCE is that they are best avoided. They are expensive, inconsistent with other tooling and many of us use our own implementations.

For alternatives (in case you don’t end up with a fix), I recommend checking out FigBug’s stack blur. There’s also a PR open for a modern StackBlur implementation.

@t0m gave a presentation last night saying improvements to the animation system and rendering in JUCE might be around the corner (exciting!). Perhaps some of these effects will be modernized along with that too?

1 Like
#3

Was that presentation public? Can it be watch somewhere?

#4

Oh, and if you do end up popping in a stack blur implementation, you can then render shadows with paths, here’s my implementation. Just needs to be called before other drawing in the paint method. For extra efficiency on a fixed size “modal” or menu, you can perform the blur in a wrapper component and call setBufferedToImage(true) so the shadow rendering is cached.

2 Likes
#5

It’s the last presentation here: Audio Programmer Virtual Meetup - June 14th @ 18:30 BST - YouTube

1 Like
#6

Great! Thanks a lot!

1 Like
#7

I downgraded to 6.1.2 and the UI performance in debug builds is back. Profiling juce::DropShadower::update shows up way down in the list at 0.59% CPU usage - a factor of 100 less (except of course that the CPU it consumed before was degrading that available to the rest of the application).

Conclusion? This ‘fix’ in 6.1.6 that you pointed to is a significant performance problem and has impacted my productivity significantly. Looking at the code change the recursion it unleashes appears to add a very significant overhead to even simple rendering. In a release build it is not as significant - but it is still there.

Dev team - suggest you take a look - reporting to github.
([Bug]: Dropshadower impact on performance (JUCE 6.1.6 from 6.1.2) ¡ Issue #1080 ¡ juce-framework/JUCE ¡ GitHub)

#8

That’s not it. This is the commit that puts the timer and the call to isWindowOnCurrentVirtualDesktop, fixing this bug. It was followed by this and this, fixing this subsequent bug. Basically, in a quite corner case you could get your drop shadows floating alone in the wrong desktop. The issue seems to be that Windows doesn’t notify when the user has moved to another virtual desktop, so you have to ask it regularly, hence the timer callback doing just that. It’s a 5 Hz timer, so it shouldn’t impact performance noticeably, but I recall from working on the subsequent bug that the system call itself is quite costly in Debug, getting heavily optimized in Release.

1 Like
#9

Mmm… code can be optimized, but how can “a system call” get optimized?

Sorry if I misunderstand your last sentence.

#10

That makes sense, and I don’t have an answer -this involves a couple COM calls and I don’t know enough about COM to say anything about it. I just recall this difference between Debug and Release centered on the function that makes these COM calls and nothing else, just like it’s reported now. Then again, if I check with my stuff I get less than 0.05% for this call in Debug, but the only drop shadow I have is the default one for the whole window, so the use case may not be comparable.

#11

Thanks for the insight. I see the 5Hz timer however it is spending a very long time inside that call. And also curious how on a release build this overhead goes away… Also regarding the drop shadows - I am not doing anything special and (with a few minor changes) just using the default LookAndFeel. I believe that this is drop shadowing a lot of things. PopupMenus in 6.1.6 are taking forever - even regular dialogs are very unresponsive. Seems like the edge case fix has put a performance issue front-and-centre. To maintain my release schedule I have reverted to 6.1.2.

#12

I’ve checked again, and there doesn’t seem to be any difference between Debug and Release -probably the message thread is just less busy. There’s something I think is just wrong: all drop shadowers are running the timer, when the issue only affects components on the desktop. Fixing that may or may not help here -it won’t affect popup menus or dialogs, for example. To be honest, the current implementation with this separate class for parent visibility is rather spaghetti-ish and hard to reason about -I think it needs refactoring. I tried to simplify it last time but there clearly was no time for broad changes. So I’m rather unwilling to take a look again, as I’m quite certain it won’t be considered.

#13

@kamedin : Thanks for your investigations!

1 Like
#14

juce_DropShadower.h (3.0 KB)
juce_DropShadower.cpp (8.3 KB)

Ok, I gave it a try. The only functional change is that the timer only runs for heavyweight components, the rest is just refactoring. I don’t expect it to make much of a difference, but anyway… just in case someone wants to test.

1 Like
#15

Thank you for looking into this. Running the timer only for desktop components seems like a sensible optimization. I’m curious to know if that provides sufficient performance improvements in the originally reported case.

#16

Probably not, IsWindowOnCurrentVirtualDesktop seems to be just slow. But I think I may have a different solution.
juce_ComponentPeer.h (25.1 KB)
juce_win32_Windowing.cpp (198.3 KB)
juce_DropShadower.h (2.8 KB)
juce_DropShadower.cpp (7.4 KB)

For the shadows, we need windows that don’t show on the taskbar, and also don’t show in all virtual desktops. App windows (WS_EX_APPWINDOW) always show on the taskbar, tool windows (WS_EX_TOOLWINDOW) show in all virtual desktops only if they are not owned. So we need tool windows owned by the shadower’s owner. addToDesktop takes a parent, not an owner: we have to bypass the setting of WS_CHILD, so I add a style flag to ComponentPeer, say

windowIsOwned = (1 << 12),

then in CreateWindow (juce_win32_windowing.cpp, line 2518)

else if (parentToAddTo != nullptr)
{
    if ((styleFlags & windowIsOwned) == 0)
        type |= WS_CHILD;
}

Now we can add the shadow windows with

addToDesktop (ComponentPeer::windowIgnoresMouseClicks
                | ComponentPeer::windowIsTemporary
                | ComponentPeer::windowIgnoresKeyPresses
             #if JUCE_WINDOWS
                | ComponentPeer::windowIsOwned,
              comp->getWindowHandle()
             #endif
             );

They won’t show on the taskbar, and also won’t show in all virtual desktops. I think this is an actual solution as opposed to a workaround: not only is IsWindowOnCurrentVirtualDesktop slow and invasive (it calls PeekMessage), we also got a delay before the shadows were shown or hidden.

A note about the “refactoring”. Having a separate class to handle parent visibility creates a back-and-forth link between two component listeners which is very simplified if it’s all done in the shadower. Given that observedComponents contains the whole hierarchy including the owner, there’s no need for lastParentComp, and updateParentHierarchy can handle all listener adds and removes. The behavior is equivalent except for componentChildrenChanged, where I say

if (owner != nullptr && owner->getParentComponent() == &c)
    updateShadows();

It seems to me that this handles z-order changes. As it was, it also reacted to changes inside the owner (between its children). I don’t see how these could affect the owner’s shadow. If they do, we can restore the previous behavior with

if (owner == &c || (owner != nullptr && owner->getParentComponent() == &c))
1 Like
#17

About WS_CHILD and the hWndParent argument of CreateWindowEx: this article explains what happens here. As the title says, “A window can have a parent or an owner but not both”. When WS_CHILD is set, the window is not top-level, and hWndParent is its parent. When WS_CHILD is not set, the window is top-level, and hWndParent is its owner. “Ownership is a relationship among top-level windows”. CreateWindowEx uses the same argument to set two different things depending on whether WS_CHILD is set or not.

(edit) Here it is in commit form… in case anyone wants to test it.

#18

Thanks for the detailed investigation. I will roll-forward (back to 6.1.6) and retest with this patch in the next few days with the code (subject of the original issue) and will report back.

1 Like
#19

juce_ComponentPeer.h (23.4 KB)
juce_win32_Windowing.cpp (191.1 KB)

Great. These are patched for 6.1.6 (the ones before, as well as the commit, were for 7.0).