-
Notifications
You must be signed in to change notification settings - Fork 163
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
Windows 8.1でマウスドラッグ時のスクロールが速すぎるので描画更新の設定を変更する #1714
Conversation
✅ Build sakura 1.0.3897 completed (commit 4c7e4fd16a by @dep5) |
すまんです。説明が「意味不明」だと感じました。
速くなったから修正する 👈問題ないです。 何故速くなった? 👈書かれてないです。問題ありです。 本当? 👈自分が見た感じ、嘘ですね。 事象は「速くなった」んではないんじゃないですかね。 不具合内容が「描画効率化の名目で、削ってはいけない画面更新まで削ってしまったこと」だとするなら全体の意味が通るし、適切な変更のように見えます。 |
クソみたいな指摘をしておくと、上記の解釈からしてコミットコメントも意味不明だと思います。 |
何故この指摘が「クソみたい」なのかというと、 |
4bfa067
to
b984950
Compare
コミットコメントを変更しました。 コメントアウトしたら正式版の挙動になるということをたまたま見つけたので、 コミットをやり直すと、いただいたコメントと対応がずれてしまうのも気になりますね。 余談ですが、#1601 は3月の変更なので appveyor にも残っておらず、 #1694 では、別の修正方法を見つけていただいたので、 ほかの方法があるのでは?と思って |
Kudos, SonarCloud Quality Gate passed! |
✅ Build sakura 1.0.3900 completed (commit e576db2fd2 by @dep5) |
総括コメントを変更していただきましたが、あまり変わってない気がしました。 状況
整理すべき課題
このPRについての感想「スクロールスピードを速くしたのを元に戻す」にこだわりたいみたいですが、 「速くした」だと、なんとなく「いいこと」に見えて「なんで対応するの?」になります。 どう思います? > @beru さん |
私が行った変更で不具合が生じるようになったという事で心苦しいですが(自分の環境では再現しなかったんです><)
等を作成して頂いた修正を取り込む前に出来れば知りたいなと思います。まぁ色々な環境で提示して頂いた対策が有効で新たな問題に繋がらないのであれば、良くわからないけど取り込むというのも有りだとは思いますが…。 という事で動作確認を含め、自分の方でもレビューを進めていきたいと思います。すみません少し時間が掛かりそうです。 |
::SendMessage(hWnd, WM_SETREDRAW, FALSE, 0); | ||
if (!m_pEditView->GetSelectionInfo().IsMouseSelecting() && !m_pEditView->m_bDragMode) { | ||
::SendMessage(hWnd, WM_SETREDRAW, FALSE, 0); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この変更で気になる点としては、ここで特定の条件時にのみ処理を呼ぶようにするというのは分かりましたが、この処理に対応する ::SendMessage(hWnd, WM_SETREDRAW, TRUE, 0);
の呼び出しを無条件で呼び出すのは、特に問題が無さそうだからそのままという事でしょうかね?
あとステータスバーの描画(の抑制)がマウスドラッグ時のスクロールスピードに影響するというのも不思議な話ですね。従来は描画処理による副作用でスクロールが遅くなっていたという事なんでしょうか?
スクロール速度(スクロール量?)がどのように決まるのかが今頭の中に入っていないのでコードを見てみます。確かタイマーで行っていたような気が…。
テスト方法です。 3つのサクラエディタを用意します 文書を開きマウスで選択開始します。 スクロール開始時点のスピード Windows 10 1万行あるファイルでテストしました。 1万行のスクロールにかかる時間 Windows 10 開始時のスクロールスピードの速さが気になっていましたが、マウスカーソルをさらに下(上)に移動する それぞれ違うマシンですが、3つのサクラエディタの比較にはなると思います。 |
berryzplusさん beru さん ::SendMessage(hWnd, WM_SETREDRAW, TRUE, 0); 従来のほうがスクロールが遅かった可能性もあるんですね。 #1699 で画面の乱れは解決したので、スクロール量を設定できれば、そのほうがいいですね。 Windows 10でのスクロールの速さは操作できないというほどでもないですが お二人とも問題を整理してくださるので大変助かってます、ありがとうございます。 |
自分はWindows 10でしか確認していませんが、ステータスバーの描画を抑制すると 自分の方で強引な対策を入れて動作確認してみました。
最近のプロセッサは高速なので、それぐらいの条件判定では重くはならないですよ。
自分もそういう理解です。マウスドラッグの量を少なめにした時に以前より滑らかにスクロールするのが感じれると思います。
マウスドラッグの量(ウィンドウからはみ出たマウスドラッグの量)に応じてスクロール速度が多少変わりますが、全体的にちょっと速すぎますね。 一定時間以内にたくさんスクロール処理が呼び出されると結構大きい数字になるのが問題のメカニズムだと思います。 対策方法は色々と考えられます。
問題提起してくれてありがとうございます。色々と問題が有っても慣れるとそれに気づかなくなったりするので人の意見はありがたいです。 |
エディットビュー上でドラッグを開始する(マウスの左ボタン押しっぱなし) 「スクロール速度が速すぎる」の対策は普通、スクロール条件とスクロール量を見直すんじゃないかと思います。 スクロール条件によっては、あるイベントに対してスクロール指示が多数発行されてしまったり、 報告されてる内容からすると、おそらく「スクロール条件が誤っている」んですよね。 「スクロール条件」や「スクロール量の決定」に関しては、 で、今回は条件を直すんでしたっけ?というと違っていて、 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
対象環境がwindows8.1ということもあり、判断できないので入れてみたらどうかと思います。
ただ、修正対象コードがキャレットを制御するクラスで、キャレットがエディットビューやステータスビューの描画を抑制したりしなかったりする現在の構成は誤っているような気がします。(なので、このPRで入れる修正はいずれ消さないといけないと思います。)
ステータスバーの再描画を というのは現象の対処方法としてはまぁ分かるんですが、ステータスバーを表示していない時には問題が無いのかが気になります。 #1714 (comment) でdep5さんがテスト方法を書いてくれたのでこちらでも確認しようと思います。 マウスドラッグの量(エディト領域からはみ出たマウスドラッグの量)に応じてスクロールの速度が変わりますが、そんなにドラッグを大きくしていないのに大きく縦スクロールしてしまう(速すぎる)というのが問題なんだと思います。 ドラッグの量に応じてスクロール量を決める処理内容は特に変えた覚えは無いので、
|
ほかの方法でスクロールを遅くできないかといろいろ触ってみましたがやっぱりうまくいきませんでした。 if (abs((Int)nScrollRowNum) < 2) nScrollRowNumが小さい時の挙動を何段階か設定すれば、 beruさんは元のコードでもスクロールスピードが落とせたのですか? #1694 (comment) でも書きましたが、改めて調べると今回のPRの内容でも ::SendMessage(hWnd, WM_SETREDRAW, TRUE, 0); 多すぎる WM_MOUSEMOVE イベントをどうにかする方向でどなたか対処していただけるとうれしいです。 |
berryzplusさん ほかの方法がありそうなので、もう少し待ってみたいと思います。 |
@dep5 さん 一定時間以内にスクロール処理が必要以上に呼び出しが多くされないように対策を入れる処理例として下記のような対策はどうでしょうか? beru@ca74f1d#diff-5388f4e469fbee9a3d53bbd2600d62a1e37badcd9e2f74ab002baf64478a62b6 |
ある程度は落とせましたがそれが理想の挙動かというと全然そんな事も無いです。 色々な環境では試していないですが、多分対策としてはあまり良くないと思います。
|
beruさんの新しいコードをテストしてみました。 if (abs(nScrollRowsPerTiming) >= 8) { CEditView_Mouse.cpp Windows 10 CEditView_Mouse.cpp Windows 8.1 CEditView_Mouse.cpp ステータスバーの表示の有無でスクロール方法をあまり変えたくないなら わたしはCCaret.cppの動きのほうが好みです。 |
@dep5 さん、テストありがとうございます。遅い返事でごめんなさい。
自分の方でこの変更を確認してみました。ちなみに下記のようなコードにしました。 if (abs(nScrollRowsPerTiming) >= 1) {
nScrollRowNum = 0;
}else
if (abs(nScrollRowsPerTiming) >= 8) {
nScrollRowNum = std::min(nScrollRowNum, (CLayoutInt)+1);
nScrollRowNum = std::max(nScrollRowNum, (CLayoutInt)-1);
} 上記のコードにするとドラッグ量が小さい場合はほぼスクロールがされないので、きちんと操作が出来ないです。おそらくdep5さんの方では違う内容の変更をしたのかもしれないですね。 なおマークダウンではコードの前後を3連続 backquote ` 文字で囲む事で見やすく表示してくれます。githubのコメント入力欄のツールバーにも <> というアイコンがあります(ショートカットは ctrl + e)。dep5さんも是非この機能を活用してください。 あとコメント中でコードだと結局断片的で分かりにくいのでテスト用のブランチを作ってコミットしてそのブランチを push した方が、手間ですが変更内容を伝えやすいと思います。例えば下記のようにです。 beru@255b4ea#diff-e14767802a1b71dcc35a08214814bac0acdce9b11dbbd41e333e64c6858a7990
動作確認ありがとうございました。
あとそもそも |
Windows 10でテストしました。 50行スクロール - 255b4ea この変更でこちらではちょうどよいスピードになりました。 255b4ea から
この部分を削除してもちょうどよいスピードのままでしたので
逆にberuさんはこれの追加でスクロールが遅くなったのですね。 二つともうまくいかせる表現があるといいんですが「1以上」のほうが優先されちゃいそうですね・・・ 何が影響しているんでしょうね <>を使うと見やすくなりますね。ありがとうございます |
画面解像度が違う、という結論になってる気がします。 MouseMoveで認識できる移動量はpx単位ですが、 1280x768な画面の場合、1行あたりのピクセル数は・・・ 元々のコードは96DPI換算の移動量になってるので、 |
@dep5 さん 動作確認してくださっているのはとてもありがたいのですが、コメント( #1714 (comment) )にdep5さんが変更したコードが断片的に紹介されているので、他の人がその断片的なコードをコピーして手動でマージすると、dep5さんが確認したソースコードと内容が食い違う可能性が出てきます。 手間だとは思いますがdep5さんの方でブランチを作成してそれをgithubにpushしてそのURLを伝えていただくのが良いかなと思います。 |
これ、なんでしたっけ? ・・・。 #1714 (comment) で「別案を検討しよう、という流れになった」とあるので閉じておきます。 |
beruさん https://github.com/dep5/sakura/compare/fixscrollspeedwin81 |
@dep5 さん、リンクありがとうございます。動作確認してみます。 |
この件って結局、「Windows 8.1で~」なんですかね? 当初気付いてなかったんですが、 「Windows 8.1で~」という問題なのであれば、 |
beruさん |
berryzplusさん |
PR の目的
マウスドラッグ時のスクロールスピードが速くなったようなので修正します
カテゴリ
PR の背景
Windows 8.1で、マウスで選択中または選択部分をマウスでドラッグアンドドロップするときに
スクロールが始まる領域までマウスを少しずつ動かしても、高速にスクロールしてしまいます。
数秒で数百行スクロールするので操作できません。
Windows 10では数十行ほどなのでウィンドウサイズが大きければ操作できます。
正式版では一行程度のスクロールができました。
PR のメリット
ウィンドウに表示されてない範囲まで選択する際に、微調整できるようになります。
PR のデメリット (トレードオフとかあれば)
仕様・動作説明
view/CCaret.cppの
::SendMessage(hWnd, WM_SETREDRAW, FALSE, 0);
をコメントアウトすると収まったので、「PRの背景」に書いた条件時に呼び出さないようにします
PR の影響範囲
テスト内容
テスト1
手順
関連 issue, PR
#1601
#1694
参考資料