Opened 14 years ago
Closed 12 years ago
#7525 closed enhancement (Won't Fix)
Optimize Pulse handling in MythUI
Reported by: | danielk | Owned by: | Jim Stichnoth |
---|---|---|---|
Priority: | minor | Milestone: | unknown |
Component: | MythTV - User Interface Library | Version: | head |
Severity: | medium | Keywords: | |
Cc: | beirdo | Ticket locked: | no |
Description
There are three things wrong here:
- Both HandleMovementPulse? and HandleAlphaPulse? are dependent on
being called at a particular frequency to create a smooth animation. Instead the alpha and movement should progress based on time elapsed.
- Both HandleMovementPulse? and HandleAlphaPulse? unconditionally request a repaint, even if the alpha and/or position is unchanged.
- Pulse() is called for all children, whether they need a pulse or not. Only children needing a pulse or containing children needing a pulse should be called. If UI elements need a pulse the pulse time should be shut-off.
Attachments (4)
Change History (14)
comment:1 Changed 14 years ago by
Changed 14 years ago by
Attachment: | mythui_pulse_optimize.patch added |
---|
comment:2 Changed 14 years ago by
Status: | new → assigned |
---|
comment:3 Changed 14 years ago by
Milestone: | unknown → 0.24 |
---|---|
Type: | task → enhancement |
Changed 14 years ago by
Attachment: | mythui_pulse_optimize_v2.patch added |
---|
comment:5 Changed 14 years ago by
Cc: | beirdo added |
---|
Changed 14 years ago by
Attachment: | mythui_pulse_optimize_v3.patch added |
---|
comment:6 Changed 14 years ago by
Updated with v3 patch. The previous versions had a bug where Pulse() was called on children only if some child has a custom pulse set. This e.g. cause the "Recording..." text in the Arclight PBB not to be alphapulsed.
The patch still has a potential problem when m_Visible is changed from false to true. In this case, the whole subtree should probably be reexamined to see what needs to be pulsed. I haven't seen any actual problems so far, though.
Changed 14 years ago by
Attachment: | mythui_pulse_optimize_v4.patch added |
---|
comment:7 Changed 14 years ago by
Milestone: | 0.24 → 0.25 |
---|
While this does provide a benefit, it is a bit invasive to be applying during feature freeze.
comment:9 Changed 12 years ago by
Milestone: | 0.25 → unknown |
---|---|
Owner: | changed from danielk to Jim Stichnoth |
Jim, I'll leave it up to you whether to commit post 0.25 freeze or abandon.
comment:10 Changed 12 years ago by
Resolution: | → Won't Fix |
---|---|
Status: | assigned → closed |
To my recollection, this ticket was created as a result of the often-reported complaint of 7% or 14% CPU usage when the frontend is "idle", before the real reason was discovered in #7953.
Given that this is no longer a problem, the fix to issue 3 adds too much complexity for too little performance gain. See the discussion in http://www.gossamer-threads.com/lists/mythtv/dev/448052 for an idea of the problems that came up.
Issues 1 and 2 (especially issue 1) may be interesting for a non performance related result, but that can be in a new ticket.
I implemented the third item -- only traverse children that need to pulse. There are actually three independent pulse attributes: movement, alpha, and custom. Each attribute is tracked as a boolean indicating whether a pulse check is needed for the next "frame". Any time the pulse state changes (e.g. start/stop movement/animation) or a child is added/deleted, the state change is propagated up the tree. The top-level MythUIType::Pulse() method always clears the custom pulse attribute, so any overridden Pulse() method needs to explicitly set the custom pulse attribute appropriately *after* calling MythUIType::Pulse().
I'm attaching the patch, but keep in mind that after applying the patch in #7953, idle frontend CPU utilization on a lowly ION frontend is down to about 1%, maybe as high as 2%, so there's not a whole lot of performance left to be gained.