Repository navigation
Defer cursor winrt event triggering - #10685
2 commits merged into
Conversation
|
The winrt event is shockingly expensive... Before:
After:
|
|
Love it! Just a quick question: Is this thread safe? (I'm unfamiliar with the Cursor class.) Also I believe it's kind of backwards that we use a |
|
I find it weird that we have to avoid sending this on the sender side and aren't appropriately coalescing or discarding the additional notifications on the receiving side. But I'm willing to take whatever works.
Leonard Hecker (@lhecker), last I checked... the renderers DO have a Begin and End draw sort of thing... can you clarify further what you mean? |
|
Michael Niksa (@miniksa) An eventing system based on when the renderer draws, allows us to keep any other UI modifications in sync. For instance instead of using a throttled func, we could hook into the render loop using events and update the scrollbar in sync with when the renderer pushes pixels on the screen, without having to use any additional timers for that. |
|
I don’t actually know if there’s a way to disable the event completely on the receiver side. We already have throttle_func as sort of “disabling” it, but that’s not good enough. The WinRt event is just so expensive that we can’t afford. I think Leonard Hecker (@lhecker)’s idea is actually how UI framework usually works, for example that’s basically how “Runloop” in iOS works. The problem here is that we have two independent “framework” here, one being the DxRenderer and related, the other being XAML. And the main way for them to communicate was through WinRT events. I see this as another consequence of tight-coupling of renderer.cpp (where the render thread lives) and DxEngine. After #10615 (seriously guys, take a look and you can see how effective that PR is) and future PR lands, we should be able to use the “hooking into the render thread” way to do something interesting. Of course the UI thread limitation in XAML still need to be taken into consideration. |
Dude its in my queue, it's just a big deal so I've been procrastinating. I feel seen. (I plan to look Monday... you're just catching me feeling too sleepy to ponder long term consequences...) |
OK well I'm willing to take this until we get further along if it helps this much. Just leave commentary. |
Mike Griese (zadjii-msft)
added
Area-Performance
Mike Griese (zadjii-msft)
added
the
AutoMerge
|
Hello Mike Griese (@zadjii-msft)! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
|
🎉 Handy links: |
I can find the commit where this regressed tomorrow if needed. In #10685 we stopped emitting these notifications while we were deferring cursor drawing. We however forgot to send the notification at the end of the defer. This likely has a small perf impact. We however do need these cursor position events for IMEs to be able to track the cursor position (and low key for #10821) * [x] regressed in #10685 * [x] Closes #11170 * [x] I work here * [x] Tests added
I can find the commit where this regressed tomorrow if needed. In #10685 we stopped emitting these notifications while we were deferring cursor drawing. We however forgot to send the notification at the end of the defer. This likely has a small perf impact. We however do need these cursor position events for IMEs to be able to track the cursor position (and low key for #10821) * [x] regressed in #10685 * [x] Closes #11170 * [x] I work here * [x] Tests added (cherry picked from commit 01fd714)
Summary of the Pull Request
References
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed