DYN-8305: Implement Bitmap Cache for nodes when zoomed out #16174
Conversation
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8305
| //disable bitmap caching if max zoom scale set to 0, or feature flag was unable to fetch; | ||
| if (ViewModel.MaxZoomScaleForBitmapCache == 0) return; | ||
|
|
||
| if (!ViewModel.StopNodeViewOpacityAnimations) |
There was a problem hiding this comment.
Why is this bitmap cache being tied to this other animation property?
There was a problem hiding this comment.
Graphs with lower number of nodes, does not really require this optimization, so it makes sense to trigger this beyond a certain number of nodes. In this case the animation property also uses another feature flag that is set to 150, which we are re-using in this case.
There was a problem hiding this comment.
what do you think about renaming this property?
There was a problem hiding this comment.
How about NodeCountOptimizationEnabled ? (As suggested by copilot)
| return; | ||
| } | ||
|
|
||
| var newRenderScale = newZoomScale <= .2 ? .2 : newZoomScale <= .5 ? .5 : 1; |
There was a problem hiding this comment.
I thought this was a bit hard to read - copilot suggests:
var newRenderScale = newZoomScale switch
{
<= 0.2 => 0.2,
<= 0.5 => 0.5,
_ => 1
};
which I think looks a bit simpler?
|
|
||
| private void ClearNodeViewCache() | ||
| { | ||
| var nodes = this.ChildrenOfType<NodeView>(); |
There was a problem hiding this comment.
hmm - have you tested this change on a custom node workspace?
| BitmapCache sharedCache = null; | ||
| foreach (var node in nodes) | ||
| { | ||
| if (node.CacheMode is BitmapCache cache) |
There was a problem hiding this comment.
is this just a null check?
There was a problem hiding this comment.
it might be more clear if you just check it's null.
There was a problem hiding this comment.
We need to cast anyway if the condition is true, I think it is better since it checks for null and also gives us the appropriate object
| internal T CheckFeatureFlag<T>(string featureFlagKey, T defaultval) | ||
| { | ||
| if(!(defaultval is bool || defaultval is string || defaultval is long)){ | ||
| if(!(defaultval is bool || defaultval is string || defaultval is long || defaultval is double)){ |
There was a problem hiding this comment.
comment is now out of date on this method.
Purpose
The PR is based on changes here
changes from the original PR:
As described in the original PR,
It introduces a mechanism by which, when the number of nodes on canvas is higher than a certain threshold(150) and zoom scale is at a certain level , we replace all node views with rendered bitmaps, using WPF's existing mechanism called BitmapCache.
Based on the current zoom, we can vary the resolution of the images and create a pseudo Load On Demand system that switches between a low, medium, high and native scale.
So at the zoomed out state we load node views at 0.2 scale (low res), then as zoom increases we switch to 0.5, and then to native scale.
Improved graph movement:
Before:
After:
Profile results: When scrolling to change zoom level, the cache update and clear takes an additional ~350ms (0.3s) at certain zoom level when the bitmap kicks in, but has no significant change from previous state.
Tested with Autodesk-MaRS-office-example-v3.dyn and Entry 05_Christmas Tree with ornaments over and around.dyn graphs with identical results.
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes