Skip to content

Optimize BitFlipPanel to suppress flicker when users switch between bit lengths#640

Merged
EriWong merged 7 commits intomicrosoft:masterfrom
rudyhuyn:BitsFlip
Aug 26, 2019
Merged

Optimize BitFlipPanel to suppress flicker when users switch between bit lengths#640
EriWong merged 7 commits intomicrosoft:masterfrom
rudyhuyn:BitsFlip

Conversation

@rudyhuyn
Copy link
Copy Markdown
Contributor

@rudyhuyn rudyhuyn commented Aug 14, 2019

Fixes #408

The main cause of this flicker is the long delay between the value changes of Checkbox::IsChecked and Checkbox::IsEnabled properties.

This is due to how the calculator generates the 64 different AutomationProperties::Name values and how the app converts the number in enumerable. These 2 tasks were too CPU-intensive for no reason.

Description of the changes:

  • Optimize how we convert the binary number into a list of booleans:
    • Before:
      • In VM: call GetStringForDisplay, GroupDigitsPerRadix (add spaces), localizer.LocalizeDisplayValue (convert '0', '1' into the local equivalents)
      • In View: remove LTR, RTL, spaces, etc... , remove characters added by LocalizeDisplayValue, add padding ('0'), compare each characters with the local '0'
    • Now:
      • In VM: call GetStringForDisplay, convert the string in vector
      • in View: nothing
  • remove the very heavy and not optimized BitFlipAutomationNameConverter converter and replace it by a lighter and more optimized way to generate strings (note: this converter was called 64 times every time the number was changed or the bit length changed)
  • Create new type BitLength and use only one way to represent bit lengths in CopyPasteManager, StandardCalculatorViewModel (instead of 4 booleans + 1 enum)
  • The view model is now responsible for sending commands to CalculatorManager instead of the view.
  • optimize CalculatorProgrammerBitFlipPanel
    • remove half of native bindings
    • modify how the application manages the visibility of this control

How changes were validated:

  • Tested in all modes

Result

  • CPU usage decreased when users enter a new number or change bit length
  • No more flickering

Copy link
Copy Markdown
Contributor

@EriWong EriWong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good. I've left a few minor comments and will approve this once they get resolved. Thanks a bunch for fixing this!

@rudyhuyn
Copy link
Copy Markdown
Contributor Author

Note about the new strings:

Copy link
Copy Markdown
Contributor

@EriWong EriWong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I found another small bug in that automation name is not updating when bits are getting flipped. Aside from that, your fix looks good to go.

Copy link
Copy Markdown
Contributor

@EriWong EriWong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks for fixing this!

Copy link
Copy Markdown
Contributor

@EriWong EriWong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you have a conflict. I can accept this PR once you've resolved the merge. Thanks!

@ghost ghost removed the needs author feedback label Aug 24, 2019
@rudyhuyn
Copy link
Copy Markdown
Contributor Author

Looks like you have a conflict. I can accept this PR once you've resolved the merge. Thanks!

Done!

Copy link
Copy Markdown
Contributor

@EriWong EriWong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The programmer tab's byte view flickers when switching between byte, dword, and qword only when standard view has not been used

3 participants