ツールバーを非表示から表示に切り替える際にアイコンの状態を設定する処理を呼び出し

[beru]

キーボードの Ctrl + 1 を繰り返し押してツールバーの表示切替を行っていると、非表示 → 表示 した直後にツールバーのアイコンの見た目が無効から有効に変化する場合がある事に気づきました。

これはタイマーでツールバーのアイコンを設定する CMainToolBar::UpdateToolbar が呼び出されている為に、切替タイミングによってはまだアイコンの見た目が設定されていない状態が見えてしまう為です。

この PR では CViewCommander::Command_SHOWTOOLBAR の中においても明示的に呼び出す事で、ツールバーを 非表示 → 表示 に切り替えた直後からアイコンが適切な表示状態になるようにしました。

[m-tmatma]

1125863 で Ctrl + 1 を押し続けて試してみましたが、何が改善しているかわからなかったです。

[berryzplus]

積極的なPRありがとうございます。
コード確認して実装的な問題はなさそうだと判断しました。
一点だけ懸念があって確認をしたいので少し時間が欲しいです。

懸念の内容なんですが、描画が切り替わらない原因に心当たりがあるんです。

1. ツールバーの状態更新は 0.3 秒間隔 の windowタイマーを使っている
2. windowタイマーは待機状態（アイドル時）にAPCキューにイベントを投げる機構である
3. サクラエディタのウインドウ表示切替の実態は「生成/破棄」になっている
4. ウインドウ生成は一般的に重い処理にあたる

仮にウインドウの生成に 0.3秒以上 かかったとすると、
ツールバーウインドウを表示するよりも先に「無効化」が処理されて、
共有メモリが書き換えられる結果、つじつまの合わない状態にならないでしょうか？

この PR でやろうとしてることは、
ビジー状態の場合に再描画が遅延するケースに対し、
強制再描画を指示することでつじつまを合わせるというものだと思います。

いまんとこ、裏が取れてなくて不安なので休日にゆっくり調べてみたい感じです。

[beru]

ツールバーの生成に 0.3 秒以上かかるPCは窓から投げ捨てておきましょう。

[beru]

Ctrl + 1 を押し続けて試してみましたが、何が改善しているかわからなかったです。

押し続けるとキーリピートの速さでツールバーの表示・非表示が切り替わってさすがに変化を目で追えないと思います。

サクラエディタを起動直後だと 上書き保存, 元に戻す, やり直し, 移動履歴: 前へ, 移動履歴: 次へ のアイコンは無効状態になります。そこで、ツールバーを非表示状態から表示状態に切り替える操作をした時に、それらのアイコンの見た目が変化しないか確認してみてください。

何回か繰り返すと、有効状態から無効状態に変化するのが確認できると思います。

[beru]

仮にウインドウの生成に 0.3秒以上 かかったとすると、
ツールバーウインドウを表示するよりも先に「無効化」が処理されて、
共有メモリが書き換えられる結果、つじつまの合わない状態にならないでしょうか？

ツールバーのウィンドウ生成の流れで、共有メモリが書き換えられる、というのが自分がわかっていないところです。どこらへんでその処理が行われているか教えて頂けないでしょうか？

[berryzplus]

ツールバーのウィンドウ生成の流れで、共有メモリが書き換えられる、というのが自分がわかっていないところです。どこらへんでその処理が行われているか教えて頂けないでしょうか？

変更前41行目、変更後42行目の m_bDispTOOLBAR を指しています。
この m_bDispTOOLBAR は共有メモリ上の変数です。
コマンドが実行されたら何もチェックせずに書き換える作りになってます。

厳密には、各ツールバーボタンの有効無効切替は別の変数を見てるはずなので違うかも知れません。

[beru]

変更前41行目、変更後42行目の m_bDispTOOLBAR を指しています。
この m_bDispTOOLBAR は共有メモリ上の変数です。
コマンドが実行されたら何もチェックせずに書き換える作りになってます。

厳密には、各ツールバーボタンの有効無効切替は別の変数を見てるはずなので違うかも知れません

あ、本当ですね。どうもありがとうございます。
正に自分がコード追加したメソッド内で変える処理が書かれてますね。
ちゃんと頭にコードが入ってませんでした。

再度コメント返信します。

仮にウインドウの生成に 0.3秒以上 かかったとすると、
ツールバーウインドウを表示するよりも先に「無効化」が処理されて、
共有メモリが書き換えられる結果、つじつまの合わない状態にならないでしょうか？

一時的につじつまが合わない状態が生じたとしても後でタイマーで CMainToolBar::UpdateToolbar が呼び出されて更新されるだろうから別に気にしなくて良いんじゃないのかなって思います。

「いや、駄目だ。このPRでそれもきちんと直さないと駄目だ。」と @berryzplus さんに執拗にコメントされたら、ターバンのガキがあなたの所へ行く事を祈願します。

[arigayas]

GitHubのコメント欄ってgifアニメを貼れるので見たら、わかりやすいかもです。

[berryzplus]

GitHubのコメント欄ってgifアニメを貼れるので見たら、わかりやすいかもです。

あざっす。

これを見た感じだと、ツールバーの有効無効が設定される前に描画されちゃってるのかもです。
ツールバーのアイコンは有効状態のものをベースに作るので、作成時に有効状態で作成しているとすると、有効無効をちゃんと設定する処理が走るまでにラグが出るかもしれません。

このPRでは、有効無効処理が走るタイミングが遅れる対策として、表示したらすぐに有効無効チェックを走らせる変更なので、発生事象に対する低減効果はあると思います。

[beru]

これを見た感じだと、ツールバーの有効無効が設定される前に描画されちゃってるのかもです。
ツールバーのアイコンは有効状態のものをベースに作るので、作成時に有効状態で作成しているとすると、有効無効をちゃんと設定する処理が走るまでにラグが出るかもしれません。

元のコードの41行目～43行目を貼り付けます。

	GetDllShareData().m_Common.m_sWindow.m_bDispTOOLBAR = ((NULL == pCEditWnd->m_cToolbar.GetToolbarHwnd())? TRUE: FALSE);	/* ツールバー表示 */
	pCEditWnd->LayoutToolBar();
	pCEditWnd->EndLayoutBars();

41行目では共有メモリ上の変数を書き換えています。
42行目ではツールバーのウィンドウを作成 or 破棄します。
43行目ではツールバーのウィンドウが有効な場合は（ShowWindow で）表示します。

かなり端折った解説ですが、有効無効を設定する処理は（300 ms 間隔の）タイマーで動くので、これらの処理の実行タイミングと同期していません。

このPRでは、有効無効処理が走るタイミングが遅れる対策として、表示したらすぐに有効無効チェックを走らせる変更なので、発生事象に対する低減効果はあると思います。

アクティブな編集ウィンドウに対しては問題が解消出来ると思います。アクティブではないウィンドウについては対策入れてません。

そちらはタイマー任せにしてますが CEditWnd::DispatchEvent の MYWM_BAR_CHANGE_NOTIFY メッセージを処理しているところで CEditWnd::LayoutToolBar を呼び出しているところがあるのでその後に入れる事も可能です。というかむしろ CEditWnd::LayoutToolBar の中で作成時に CMainToolBar::UpdateToolbar を呼び出しすればいいのかもしれないですね。

[arigayas]

書き忘れてましたが、キャプチャーソフトはScreenToGifというソフトで日本語表記にも対応してます。

[beru]

GitHubのコメント欄ってgifアニメを貼れるので見たら、わかりやすいかもです。

分かりやすいですね。百聞は一見に如かず、って感じですね。
ただちょっと切替が速すぎる気がします。表示後にすこし間を置いてもらえると
確認しやすいと思います。

[arigayas]

@beru さんへ。
ウィンドウを縦に重ねた上で少し切り替えを遅くして撮り直しました。
15秒ぐらいの繰り返しです。
Ver2.3.2.0

[beru]

@arigayas さんどうもありがとうございます。
自分も ScreenToGif を使って 14002ec をビルドしたので撮ってみました。

[beru]

rebase しました。あと必要のない変更は取り消しました。

差分は1行だけなので、とても単純な変更内容だと思います。

[berryzplus]

対応ありがとうございます。LGTMです。

[beru]

レビューありがとうございました。Merge します。
問題が見つかった場合は別のPRで対処する事にしましょう。