Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

キャレット位置の文字情報をステータスバーに設定する際の再描画をまとめる #1601

Merged
merged 4 commits into from
Mar 28, 2021

Conversation

beru
Copy link
Contributor

@beru beru commented Mar 21, 2021

PR の目的

カーソルキーを押してキャレット位置を変えるとステータスバーの表示が更新されますが、その処理を効率化するのが目的です。

カテゴリ

  • 速度向上
  • 仕様変更

PR の背景

開発環境(Visual Studio)のメニューの Debug > Performance Profiler で CPU Usage にチェックを付けて Start をして、どこの処理に時間が掛かっているのかを確認する事が出来ます。

キャレット位置の文字情報をステータスバーに設定する際に呼び出される CMainStatusBar::SetStatusText における StatusBar_SetText に結構処理時間が掛かっていたので対策を行いました。

PR のメリット

キャレット位置変更時にステータスバーの更新を行う際の処理が効率化されます。

処理を効率化する事でCPU使用率が下がるのではないかと思います。

PR のデメリット (トレードオフとかあれば)

記述が増える事で処理内容が読み取りにくくなります。

元々そこまで目立って重たい処理でも無いので、変更によって処理が軽くなったかを体感できるかというと出来ません。

仕様・動作説明

StatusBar_SetText の実装は SB_SETTEXT メッセージを SendMessage 関数で送るものです。

https://docs.microsoft.com/en-us/windows/win32/controls/sb-settext#remarks

によると、

The message invalidates the portion of the window that has changed, causing it to display the new text when the window next receives the WM_PAINT message.

と書かれていたのでおそらく内部で InvalidateRect しているように思えます。

このPRの変更では WM_SETREDRAW メッセージを使う事でステータスバーの各パートに文字列を設定する際に個々に描画を行われる事を防ぎます。また各パートに文字列が設定された場合は各パートのRECT領域を取得して結合し、後で InvalidateRect を呼び出す事で再描画をまとめる事で表示処理の効率化を図ります。

描画をまとめて行う事で、カーソルキーを押しっぱなしにした場合のステータスバーの表示のちらつきが無くなるかというと、残念ながらそんな事は無いようです。
追記:拡張スタイル WS_EX_COMPOSITED を使う事でステータスバーの表示のちらつきを解消しました。

PR の影響範囲

テスト内容

テスト1

手順

  • テキストファイル(たとえば README.md とか)を開いた後にカーソルキーを押してキャレット位置を変える操作を行い、ステータスバーの表示が更新されるかどうかを確認

関連 issue, PR

#1592, #1265, #1246, #452

参考資料

https://docs.microsoft.com/en-us/windows/win32/controls/sb-getrect

https://docs.microsoft.com/en-us/windows/win32/controls/sb-settext

https://docs.microsoft.com/en-us/windows/win32/gdi/wm-setredraw

https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-invalidaterect

https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-unionrect

https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-createwindowexw

https://docs.microsoft.com/en-us/windows/win32/winmsg/extended-window-styles

@beru beru added the 🚅 speed up 🚀 高速化 label Mar 21, 2021
@AppVeyorBot
Copy link

Build sakura 1.0.3589 completed (commit c344aa247c by @beru)

@beru
Copy link
Contributor Author

beru commented Mar 21, 2021

dac4cd3#1592 と同じように拡張スタイル WS_EX_COMPOSITED を使う事でステータスバーの表示のちらつきを解消しました。

@beru beru added the specification change ■仕様変更 label Mar 21, 2021
@berryzplus
Copy link
Contributor

Code Smell B 8 Code Smells

ざっくり2件の指摘が上がっています。

  • 関数呼び出しと同時に index++ してるのをばらしたほうがよい。(×7)
  • ラムダ式が44行もあって長いので、多くても20行程度に整理すべき。

修正内容は見てるので「ええっ!」って感想も分かりますが、なるたけ対応したいです。

@beru
Copy link
Contributor Author

beru commented Mar 21, 2021

Code Smell B 8 Code Smells

ざっくり2件の指摘が上がっています。

* 関数呼び出しと同時に index++ してるのをばらしたほうがよい。(×7)

* ラムダ式が44行もあって長いので、多くても20行程度に整理すべき。

修正内容は見てるので「ええっ!」って感想も分かりますが、なるたけ対応したいです。

* 関数呼び出しと同時に index++ してるのをばらしたほうがよい。(×7)

については元のコードがそういう記述内容なのでそっとしておきたいです。
ばらす事で可読性が悪化してしまうと思いますし。

* ラムダ式が44行もあって長いので、多くても20行程度に整理すべき。

に関してはもっともな指摘だと思います。ここはラムダ式を使わずに do while ループに変更します。

@beru
Copy link
Contributor Author

beru commented Mar 21, 2021

しかし CCaret::ShowCaretPosInfo() メソッドは行数が多いですね。

このPRの本来の意図はそのメソッド等に入れた変更で再描画回数を減らして処理負荷を下げる事ですが、拡張スタイル WS_EX_COMPOSITED を付けてちらつきを解消出来た事の方が効果としては美味しいと思えてきました…。

@AppVeyorBot
Copy link

Build sakura 1.0.3591 completed (commit bd0eef7727 by @beru)

@AppVeyorBot
Copy link

Build sakura 1.0.3593 completed (commit a506268168 by @beru)

Copy link
Contributor

@berryzplus berryzplus left a comment

Choose a reason for hiding this comment

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

先日のレビュー時点に比べて警告が増えているようです。

A 11 Code Smells

前回 8件⇒11件

index++ を修正できないのは了解ですが、他は対応が要ると思っています。

@beru
Copy link
Contributor Author

beru commented Mar 28, 2021

前回 8件⇒11件

index++ を修正できないのは了解ですが、他は対応が要ると思っています。

Use the "nullptr" literal. について

WindowsAPI を呼び出している箇所で引数に NULL を指定している部分ですね。
ここで警告を出すのは間違っていると思いますが簡単に対応できるので変更しておきました。

Reduce the number of nested "break" statements from 4 to 1 authorized. について

これは do-while文を break で抜けるのを4箇所ではなくて1個所に減らすようにという指摘ですが、この対応をすると逆に可読性が低下すると思うので変更しません。

do-while 文を使っているのは lambda式が長すぎると静的解析が CodeSmell にしてきたからですが、他の方法で対処するとしたら、StatusBar_SetText 関数を呼びだすかどうかを判定する処理を関数化する手段があると思います。

Convert this integer literal to a bool literal. について

do-while文の条件式の部分で整数リテラルではなくてブーリアンのリテラルを使うように変えなさい、という指摘です。
こちらも対応するのもなんか微妙な気がしますが簡単に対応できるので変更しました。

@sonarcloud
Copy link

sonarcloud bot commented Mar 28, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 8 Code Smells

94.3% 94.3% Coverage
0.0% 0.0% Duplication

@AppVeyorBot
Copy link

Build sakura 1.0.3615 completed (commit 558789a3b1 by @beru)

@berryzplus berryzplus dismissed their stale review March 28, 2021 11:20

対応確認したので。

@beru
Copy link
Contributor Author

beru commented Mar 28, 2021

レビューありがとうございました。Mergeします。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants