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

ツールバーの状態更新を必要な場合にのみ行うように変更 #456

Merged
merged 2 commits into from
Sep 18, 2018
Merged

Conversation

beru
Copy link
Contributor

@beru beru commented Sep 17, 2018

CEditWnd::Timer_ONOFF にて 300ミリ秒間隔でツールバー更新用のタイマーが起動されて、CMainToolBar::UpdateToolbar メソッドが定期的に実行されます。

その中で行われているツールバーのボタンを設定する処理方法を変更しました。

有効無効の設定で Toolbar_EnableButton 関数が、
チェック状態の設定で Toolbar_CheckButton 関数が使われていましたが、
新規に追加した Toolbar_SetState 関数を使って一括で行うようにしました。

また事前に、新規に追加した Toolbar_GetState 関数を使ってボタンの状態を取得して、
設定が必要ない場合は Toolbar_SetState 関数を呼ばないようにしています。

無駄に処理を行わない事でCPU使用率をとてもわずかですが下げる効果が有ります。

@@ -78,6 +78,8 @@ namespace ApiWrap
inline int Toolbar_SetButtonInfo(HWND hwndCtl, int index, TBBUTTONINFO* info) { return (int)(DWORD)::SendMessage(hwndCtl, TB_SETBUTTONINFO, (WPARAM)index, (LPARAM)info); }
inline BOOL Toolbar_SetButtonSize(HWND hwndCtl, int width, int height) { return (BOOL)(DWORD)::SendMessage(hwndCtl, TB_SETBUTTONSIZE, 0L, MAKELONG(width, height)); }
inline DWORD Toolbar_SetExtendedStyle(HWND hwndCtl, DWORD styles) { return (DWORD)::SendMessage(hwndCtl, TB_SETEXTENDEDSTYLE, 0L, (LPARAM)styles); }
inline int Toolbar_GetState(HWND hwndCtl, int index) { return (int)(DWORD)::SendMessage(hwndCtl, TB_GETSTATE, (WPARAM)index, 0L); }
inline int Toolbar_SetState(HWND hwndCtl, int index, WORD state) { return (int)(DWORD)::SendMessage(hwndCtl, TB_SETSTATE, (WPARAM)index, state); }
Copy link
Member

Choose a reason for hiding this comment

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

DWORD にキャストしてから int にキャストしてますが、
DWORD にキャストしない方がいいと思います。

SendMessage は LRESULT を返しますが、LRESULT は LONG_PTR と定義されています。

C:\Program Files (x86)\Windows Kits\10\Include\10.0.10586.0\shared\wtypes.h(157,18)  [SJIS]: typedef LONG_PTR LRESULT;

Copy link
Contributor

Choose a reason for hiding this comment

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

う~む。

https://docs.microsoft.com/en-us/windows/desktop/Controls/tb-getstate

C:\Program Files (x86)\Windows Kits\8.1\Include\um\CommCtrl.h(1236):#define TBSTATE_MARKED          0x80

DWORD にキャストしてはダメな理由がないような。
逆に、キャストする理由もないような。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

e3ac9ef で修正しました。
なお、Toolbar_SetState 関数の戻り値は MSDN の記載を確認して、BOOL 型に変更しました。

Copy link
Member

Choose a reason for hiding this comment

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

DWORD にキャストしてはダメな理由がないような。

失敗したときに -1 を返すからです。

@beru beru added the 🚅 speed up 🚀 高速化 label Sep 17, 2018
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.

ぼく的にはOKです。先にやり取りがあるようなので状態は触りません。

@beru
Copy link
Contributor Author

beru commented Sep 18, 2018

先にやり取り って具体的に何でしょうか?

自分としてはレビュー指摘点に対応はしましたが、その後特に反応が無いので放置プレイなのかと思っていました。

@m-tmatma
Copy link
Member

忘れてました。明日見ます。

@berryzplus
Copy link
Contributor

明日見ます。

では代わりにOK出しときます。何かあれば後追いでもコメントお願いします。

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.

LGTMです。

時間を置くと他と被る可能性が高いクラスなので、ぼく基準+レビュー対応内容の確認でOKと判断してOKにしてしまおうと思います。

@beru
Copy link
Contributor Author

beru commented Sep 18, 2018

ではちょっと早いですが Merge してしまいます。もし問題が見つかったらあとで修正PR を作成します。

@beru beru merged commit f411bb3 into sakura-editor:master Sep 18, 2018
@beru beru deleted the UpdateToolbar branch September 18, 2018 14:36
@m-tmatma
Copy link
Member

では代わりにOK出しときます。何かあれば後追いでもコメントお願いします。

ありがとうございます。

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.

3 participants