search

dotnet / wpf

WPF is a .NET Core UI framework for building Windows desktop applications.
MIT License
7.07k stars 1.17k forks source link

Concerns and Suggestions Regarding the Use of DynamicResource in Win11 Theming for WPF Repository #8599

Open lindexi opened 10 months ago

lindexi commented 10 months ago

Dear Team,

I am writing to express my concerns about the recent changes made to the WPF repository regarding Win11 Theming, https://github.com/dotnet/wpf/discussions/8533 . I have noticed a significant increase in the use of DynamicResource in these updates.

As explained by Harshit in https://github.com/dotnet/wpf/pull/8579#discussion_r1434890994 , the purpose of using DynamicResource is to allow the interface content to change along with the theme. However, it is well-known that the use of DynamicResource comes with a substantial performance cost. This becomes particularly problematic when the entire theme style is filled with DynamicResource, as it can significantly slow down the performance of the entire application.

Furthermore, it is important to consider that users do not frequently change their system theme styles. This means that the use of DynamicResource could potentially slow down the performance for many users without bringing any significant benefits.

I have also noticed that Pankaj Chaurasia introduced the ApplicationThemeManager mechanism in https://github.com/dotnet/wpf/pull/8575/files#diff-53632fc5d46760e790532f11813877befa40746f4a8025857f3231ce07b214ee . I believe that the current WPF lacks a clear mechanism for following theme changes. In other words, relying on DynamicResource to implement theme changes seems to be an inefficient approach.

Therefore, I would like to suggest the introduction of a new mechanism in the WPF repository. This mechanism could replace the existing DynamicResource code and provide a more efficient way to implement style changes according to theme changes.

I believe that this new approach could significantly improve the performance of the application and provide a better user experience.

Thank you for considering my suggestions. I look forward to seeing improvements in the future updates.

Best regards,

miloush commented 10 months ago

While these are reasonable concerns, I would think they need to be based on real data. Harshit suggested in said comment that the performance impact is being evaluated as part of these efforts.

What is the new approach you are suggesting?

pchaurasia14 commented 10 months ago

Thanks @lindexi for sharing these concerns. Once we distill down the changes required to provide the desired look and feel, we will also evaluate the total cost of DynamicResource usage. Also, I agree, if the cost is on the higher side, we may have to introduce a 'lightweight' StaticResource that is only re-evaluated during theme changes (similar to ThemeResource in UWP).

batzen commented 8 months ago

There is an open PR from me that massively reduces the cost for DynamicResource and there is more room for improvement regarding DynamicResource. How about merging my PR?