sakura-editor / sakura

SAKURA Editor (Japanese text editor for MS Windows)
https://sakura-editor.github.io/
Other
1.25k stars 163 forks source link

タブの描画・ホイールでの選択順序の問題 #1845

Closed dep5 closed 2 years ago

dep5 commented 2 years ago

問題内容

タブバー表示時に操作に影響の出る問題が発生します。

再現手順

タブバーを表示します。

  1. 新規タブを開きます。「(無題)数字」と表示されたタブが開きます。 タブをクリックすると「(無題)」と変わり数字が消えます。 (いくつか開いた後閉じると終了時に時間がかかっています。)

  2. 「設定」「共通設定」「タブバー」「マウスホイールでタブ切替」を有効にします。 新規タブをいくつか開いた後、タブ上にマウスを移動し、ホイールを下に回します。 タブが切り替わるごとに上のようにタブのテキストが変化します。 今度はホイールを上に回すと最近開いたタブ2つだけが切り替わり続けます。 また下に回すと先ほどのタブだけ順序が変わってしまい、左から順番に開けなくなります。 上下に回しているうちに左から開くはずの順番がめちゃくちゃになってしまいます。 タブを5個開いたとしてホイールを 下に回す1234512345… 上に回す3434343434… 下に回す1243512435… といった感じです

  3. https://github.com/sakura-editor/sakura/issues/1844#issuecomment-1134149122 kazasakuさんの話とも似ているのですが タブを切り替えると閉じたはずのタブが残っていたり(選択しても何も起きない) タブを切り替えるごとにタブの表示順やタブの数が異なるということもありました。 …が、これは2/11以前からあったような気もします…

再現頻度

3はいつの間にかなっていますが、1と2は確実に起きます。

問題のカテゴリ

環境情報

スクリーンショット

dep5 commented 2 years ago

git resetしながらさかのぼったところ 2022/2/11の変更でこの現象が起きているようです。 ためしにソースを最新にしてから

cd mydir
powershell Invoke-WebRequest https://github.com/sakura-editor/sakura/pull/1794.patch -OutFile 1794.patch
git apply -R 1794.patch

としてビルドしたところ以上の症状はなくなりました。 PRのどの部分が影響しているのかまでは調べられませんでした。

usagisita commented 2 years ago

sakura_core\env\CAppNodeManager.cpp BOOL CAppNodeGroupHandle::AddEditWndList( HWND hWnd ) ・( 181,15 ): if( 0 == ::GetWindowLongPtr( hWnd, sizeof(LONG_PTR) ) ) ・( 184,7 ): ::SetWindowLongPtr( hWnd, sizeof(LONG_PTR) , (LONG_PTR)pShare->m_sNodes.m_nSequences ); hWndは編集ウィンドウなのでEditWndなのですが HWND CEditWnd::_CreateMainWindow(int nGroup, const STabGroupInfo& sTabGroupInfo) wc.cbWndExtra = 0; 1794で32バイトあったWndExtraが0にされてしまっているので、読み書きに失敗している、のでしょうね。

ghost commented 2 years ago

@dep5 @usagisita

情報ありがとうございます。僕が #1844 で言及したのはこのissueの本文中①と③です(②は初耳です)。 僕の書いたほうを重複とみなして閉じておきました。

ここに sizeof(LONG_PTR) を指定しているので謎の領域ではありません。よって、「誤記を訂正する」という趣旨の提案であれば受け入れられないと思います。「ユーザーデータを出し入れする意図ならそう書けばいい」というお話は分かりますので、GWLP_USERDATA への変更と同時に上記の指定値を0にするならば承認させていただきます。

勉強になりました。 WNDCLASSEX::cbWndExtra に指定するのは、ユーザーデータのサイズだと思ってました。 デフォルトでプライマリのユーザーデータ領域 8 バイト(32bitビルドでは4バイト)が確保されるのであれば、そこも不要だと思います。

GetWindowLongPtr(hwnd, 0)としているのは CEditView のみで、 WNDCLASSEX::cbWndExtraに必要サイズを指定しているのは他ウインドウにも当てはまるので、 同時に修正すると変な感じになると思いますがいかがでしょう?修正いれときます。

  • CEditViewの問題
    • プライマリのユーザーデータ領域を使う意図が明らかなのに拡張領域を使っている。
    • プライマリのユーザーデータ領域しか使わない場合、拡張領域のサイズを指定する必要はないがサイズを指定している。
  • 他のウインドウにも当てはまる問題
    • プライマリのユーザーデータ領域しか使わないのに、拡張領域のサイズを指定している。

Originally posted by @berryzplus in https://github.com/sakura-editor/sakura/issues/1794#issuecomment-1036014324

上記のような経緯で fa5f9a2f4034135b979a2cee08c6d223515f54a4 がコミットされていますが、拡張領域を利用する箇所が存在したということなのでしょうか。

dep5 commented 2 years ago

1794 の変更を元に戻してしばらく使っていますが、1番目2番目の問題は解消し、

今のところ3番目の現象も起きていません。 もっと前からあったように思いましたが、これが原因だったんでしょうか…?

dep5 commented 2 years ago

usagitaさん 詳しい説明ありがとうございます。 wc.cbWndExtraの値の変更の影響なのですね。

1794 まるごと元に戻す必要は無さそうですね。

ビルドする時間ができたら試してみたいと思います。

kazasakuさん 保存時のタイトルについては気づかなかったです。 タブの問題と同時に修正されるといいですね。

dep5 commented 2 years ago

テストしてみましたが、 window/CEditWnd.cpp の wc.cbWndExtra だけの変更でも タブの描画は正常になるみたいですね。

作者の方の対応を待つことにします。

dep5 commented 2 years ago

https://github.com/sakura-editor/sakura/pull/1850#issuecomment-1144647769 (レベル2) を適用して使っていますが、 ホイールを回した場合を含めて、すべての問題が解消していると思います。