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

メモリDCを利用しない場合はアンダーライン描画を行描画の直後に行う事でちらつきを抑える #1616

Merged
merged 2 commits into from
Apr 22, 2021

Conversation

beru
Copy link
Contributor

@beru beru commented Apr 3, 2021

PR の目的

共通設定の全般の「画面キャッシュを使う」にチェックを入れていない場合に、PageUp/PageDownキーを押してスクロールする際にアンダーライン描画がちらつく現象が起きます。それを回避するのが目的です。

カテゴリ

  • その他の問題

PR の背景

共通設定の全般の「画面キャッシュを使う」にチェックを入れていない場合はメモリDCが使われません。メモリDCを使わないと描画途中の絵が画面に表示される事がありちらつきが起きやすいです。

このPRでは、アンダーライン位置の行の描画直後にアンダーライン描画を行う事で、同じ位置にある要素が近いタイミングで描画されるように処理を調整してちらつきが起きにくいようにしています。

PR のメリット

表示のちらつきが減る

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

ちらつきを減らす対処を入れた分のコードの記述が増えてしまう。

仕様・動作説明

PR の影響範囲

アンダーライン描画に影響します。

共通設定の全般の「画面キャッシュを使う」にチェックを入れている場合の動作は従来と変わりません。

テスト内容

テスト1

手順

  • 行数が多いファイル、例えば sakura_core\CGrepAgent.cpp を開く
  • PageUp/PageDownキーを叩いてスクロールする操作を繰り返す
  • スクロール時にアンダーラインの表示にちらつきがおきるか目視で確認する

@AppVeyorBot
Copy link

Build sakura 1.0.3635 completed (commit 5a39b81299 by @beru)

@berryzplus
Copy link
Contributor

PR説明を読んで分からなかったことが2つあります。

  1. 「ちらつく」の原因
  2. 「メモリDCを利用しない場合はアンダーライン描画を行描画の直後に行う」にちらつきを抑える効果がある理由

アンダーセンの描画を行描画の直後に行うように「変更する」ってことは、
もともとは行描画の直後に「なんか別なこと」をしていたわけですよね。
だから、センの描画タイミングが一瞬遅れるような状況が発生して「ちらつき」に見えていたのかも。

あとは、メモリDCを使わない、にしている人が期待する動作がどうであるのかが気になります。

メモリDCってのは、描画を最適化して高速に画面表示を行うためのオプションです。
メモリDCを使わない場合、当然描画速度は落ちてしまい、表示速度は低速になります。
表示が低速になると、パラパラマンガのコマが飛んだときのような状況が発生します。
「ちらつき」っていうのは、ここでいう「コマ落ち」に近いものです。

と考えると、メモリDCを使わない場合に画面がちらつくことがあるのは、仕様じゃね?と思えてきます。

コマ落ちの原因が描画手順がマズいことに起因するなら、それは直したほうがいいと思います。
その場合、メモリDCを使う場合に影響ありません、にはならないはずで、「う~む」と思っています。

@beru
Copy link
Contributor Author

beru commented Apr 4, 2021

PR説明を読んで分からなかったことが2つあります。

1. 「ちらつく」の原因

2. 「メモリDCを利用しない場合はアンダーライン描画を行描画の直後に行う」にちらつきを抑える効果がある理由

「ちらつく」というのがどういう現象かというのをどうわかりやすく表現するかを過去に考えて #1592 (comment) で書いたのでコピペします(問題が起きてrevertしたPRでの発言なので説得力に乏しいですが…)。

ソフトウェア側が色々と描画を行いますが、きりの良いタイミングで表示の更新が行われない場合に、本来は表示するべきではない描画途中の絵が一時的に画面表示されてしまい、それをユーザーがちらつきと感じるんだと思います。

他の言い方だと、不自然なモニタの明暗の変化が短時間に起きるのがいらつきの原因になります。:angry:

アンダーセンの描画を行描画の直後に行うように「変更する」ってことは、
もともとは行描画の直後に「なんか別なこと」をしていたわけですよね。
だから、センの描画タイミングが一瞬遅れるような状況が発生して「ちらつき」に見えていたのかも。

「なんか別なこと」というのは「後続行の描画」と「ルーラー描画」です。そのうちの「後続行の描画」にある程度時間が掛かるのが要因だと思います。空行が連続するテキストだと同様のPageUp/PageDown操作を行ってもアンダーライン描画のちらつきは殆どおきません。

あとは、メモリDCを使わない、にしている人が期待する動作がどうであるのかが気になります。

メモリDCってのは、描画を最適化して高速に画面表示を行うためのオプションです。
メモリDCを使わない場合、当然描画速度は落ちてしまい、表示速度は低速になります。

メモリDCを使わない場合は WM_PAINT メッセージ処理時に BeginPaint の戻り値で取得する画面DCを直接使って描画します。画面DCに直接描く方法だと描画途中のまだ描画しきっていない内容が途中の絵が画面に一時的に表示されてしまうようでこれがちらつきの原因になるという理解です。

EndPaint の呼び出しが終わるまで実際の画面表示を待っててくれれば良いような気がしますが、画面DCを使う場合はそうはいかないようですね。なんで待ってくれないのかというと大昔はオフスクリーンバッファをデフォルトで持てるほどメモリ容量に余裕が無かったので、GDIは画面DCに描画した内容はすぐに画面表示する作りにしていたんだと思います。

Desktop Window Manager (DWM) 側ではオフスクリーンバッファを持っていると思うので待ってくれていたらなと思うんですが、そこは互換性の為なのかそういう処理はしてくれないようです。なおDWMやDWMとGDIの連携内容については全く調査しておらずわかりません。

GDIの話に戻りますが、メモリDCは CreateCompatibleDC で作成されるもので表示用のDCではなくて描画用のDCです。CreateCompatibleBitmap で作成したDDB(Device Dependent Bitmap)を SelectObject でメモリDCに関連付けることで、メモリ上のビットマップに描画が行われます。このメモリ上のビットマップに描画した内容を画面DCに一気にコピーするのが BitBlt の呼び出しです。描画し終わった内容を一度の BitBlt の呼び出しで一気に画面DCにコピーするので表示のちらつきが原理的に発生しにくくなっています。

表示が低速になると、パラパラマンガのコマが飛んだときのような状況が発生します。
「ちらつき」っていうのは、ここでいう「コマ落ち」に近いものです。

と考えると、メモリDCを使わない場合に画面がちらつくことがあるのは、仕様じゃね?と思えてきます。

コマの作画途中の絵が一時的に表示される、がより正しい表現ですかね。

メモリDCを使わない場合でも画面要素の描画順を調整する事で画面表示のちらつきを減らせる事を確認したのでPRを作りました。

コマ落ちの原因が描画手順がマズいことに起因するなら、それは直したほうがいいと思います。
その場合、メモリDCを使う場合に影響ありません、にはならないはずで、「う~む」と思っています。

メモリDCを使う場合の描画手順は元のままにしています。メモリDCを使う場合と使わない場合で処理を変えているのでソースコードの記述が増えてしまっているのは良くない点だと思います。

なおメモリDCを使う場合は描画途中の絵が画面に表示されないので、メモリDCを使わない場合と同じように描画順を変えても問題は起きないと思います。それならばメモリDCを使う場合でも使わない場合でも同じ描画手順で揃えた方が良いかもしれないですね。

共通設定の「画面キャッシュを使う」設定を有効にして使えば良いんじゃないの?と言われたら反論しにくいんですが、この設定を有効にすると少し処理が重くなる箇所もあるんですよね。例えばスクロールバーをドラッグしてスクロールする際に CEditView::ScrollDrawBitBlt の呼び出しが行われる部分です。まぁ気にするだけ損ってやつかもしれませんが…。

@berryzplus
Copy link
Contributor

一旦整理。

1.「ちらつく」の原因
👉 描画手順がマズいから。
全部の行を描画しきってから下線を引いてる。
行が沢山ある場合、行描画と下線描画のタイムラグが大きくなり、ちらつきに見える。
2.「メモリDCを利用しない場合はアンダーライン描画を行描画の直後に行う」にちらつきを抑える効果がある理由
👉 想定する原因への対処なので効果はあるはず。
3. 気になる点に関するコメント。

  • メモリDCを使わない場合に「ちらつく」のは仕様じゃね?
    👉 ちらつかせずに描画できる方法を見つけたのでPRしました。(意訳)
  • 「描画手順がマズい」を修正するならメモリDCを使う場合も修正するのが筋じゃね?
    👉 メモリDCを使う場合と使わない場合のコードが分かれているので、必要な方だけ修正します。(意訳)

で、approveしようとしたんですが「メモリDC を使わない場合だけに影響する修正」には見えませんでした。
普通にメモリDCを使う場合にも影響する修正に見えます。
「影響がない」のと「目に見える変化がない」は違うように思います。

本当に影響がないかどうかはちゃんと見れていませんが。

@beru
Copy link
Contributor Author

beru commented Apr 4, 2021

で、approveしようとしたんですが「メモリDC を使わない場合だけに影響する修正」には見えませんでした。
普通にメモリDCを使う場合にも影響する修正に見えます。

具体的にどこの差分を見てそう思われましたか?

@@ -844,16 +855,15 @@ void CEditView::OnPaint2( HDC _hdc, PAINTSTRUCT *pPs, BOOL bDrawFromComptibleBmp
pPs->rcPaint.top,
SRCCOPY
);
// From Here 2007.09.09 Moca 互換BMPによる画面バッファ
// アンダーライン描画をメモリDCからのコピー前処理から後に移動
if ( m_pcEditWnd->GetActivePane() == m_nMyIndex ){
Copy link
Contributor

Choose a reason for hiding this comment

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

修正3箇所中の2個目、ここがメモリDCを使う場合の処理に見えます。

  1. メモリDCを使わない場合の処理を追加。
  2. スコープ外にあった処理を移動(追加)
  3. スコープ外にあった処理を移動(削除)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

コメントありがとうございます。ちょっと紛らわしいですが、2. と 3. は合わせて移動的な変更でした。
メモリDC使わない場合のアンダーライン描画を行描画ループの中に追加したので、元のアンダーライン描画はメモリDCを使う場合にのみ実行するようにしました。そしてちょうど上の方にそのif文の記述があったのでそのスコープ内にさっさと引っ越しさせました。

@suconbu
Copy link
Member

suconbu commented Apr 4, 2021

変更後の動作を見ていたところ、タイプ別設定で「カーソル位置縦線」を表示させる設定をした状態で PageUp/Down 操作をすると、カーソル行より下にある縦線が消えてしまいました。
CEditView::CaretUnderLineON の中では縦線も描画しているようなので、カーソル行以降の行描画によって上書きされている気がします。

@berryzplus
Copy link
Contributor

berryzplus commented Apr 4, 2021

で、approveしようとしたんですが「メモリDC を使わない場合だけに影響する修正」には見えませんでした。
普通にメモリDCを使う場合にも影響する修正に見えます。

具体的にどこの差分を見てそう思われましたか?

単に「メモリDCを使わない場合には影響しない」と書いてあったことに引っ掛かっていました。(これはぼくの気のせいかもしれません。

共通設定の全般の「画面キャッシュを使う」にチェックを入れている場合の動作は従来と変わりません。

今見たら「動作は変わらない」になってたので問題なしです。

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.

Code Smell B 4 Code Smells

少なくとも2件は今回追加(移動)したコードで発生したもので、容易に対処可能だと思います。
最初の2件は「ぼく知らねっす」で合意。

@suconbu
Copy link
Member

suconbu commented Apr 4, 2021

変更後の動作を見ていたところ、タイプ別設定で「カーソル位置縦線」を表示させる設定をした状態で PageUp/Down 操作をすると、カーソル行より下にある縦線が消えてしまいました。

PageUp/Down で移動した先のキャレット位置に括弧がある (=対括弧の強調表示がされる) と、その場合はなぜかこの縦線が消える現象は起きませんでした。

@berryzplus
Copy link
Contributor

PageUp/Down で移動した先のキャレット位置に括弧がある (=対括弧の強調表示がされる) と、その場合はなぜかこの縦線が消える現象は起きませんでした。

対括弧の描画は、通常の描画とは別になっています。
行全体を再描画させる実装になってたように思うのでその挙動で正しいと思います。

@usagisita
Copy link
Contributor

行間を0以下にすると、カーソル行アンダーラインの位置が一つ下の行に含まれるようになるため、レンダリングの対象行が変わってしまい、PageUp/PageDownなどでアンダーラインが消えてしまうようです。

@berryzplus
Copy link
Contributor

行間を0以下にすると、カーソル行アンダーラインの位置が一つ下の行に含まれるようになるため、レンダリングの対象行が変わってしまい、PageUp/PageDownなどでアンダーラインが消えてしまうようです。

ん?

「行間を0以下にすると」
 👆この設定をできるのは「正しい」ですか?

これが正しいとすると、ちらつきを抑えるのは無理だね、な結論になります。
正しくないとすると、このPRか別のPRで「正しくない設定ができてしまう」に対策する感じになります。

個人的には正しくないんじゃね?と思っています。
(そうすることによるメリットを思いつかないから。)

@berryzplus
Copy link
Contributor

どっちにしても指定桁縦線の描画に問題が出ることが分かったので、
SonarCloudの指摘対応以外に「ちょっと重めの追加修正が必要」と判明した感じです。

@usagisita
Copy link
Contributor

僕は原理主義者ではないので「正しいか」は知りませんが……
ちなみに僕のコメント内容は「未満」ではなく「以下」で正しいです。
恐らくオリジナルのころから行間「0」は設定できます。この時点で下の行に線が引かれます。
マイナス値を許容するようになったのは、プロポーショナルフォント対応あたりからです。
https://sakura-editor.github.io/bbslog/sf/unicode/2350.html
google先生に聞いたところ、これですね。

@usagisita
Copy link
Contributor

指定桁縦線

カーソル位置縦線ではアルマイカ

@ghost
Copy link

ghost commented Apr 5, 2021

マイナス値を許容するようになったのは、プロポーショナルフォント対応あたりからです。

66f5d68 (patchunicode#713)でGetDlgItemIntの第4引数(符号付き整数とみなすかどうか)をTRUEに変更しています。
Wordでメイリオを使うときは行間を”0.87倍”に設定する必要があるのですが、このような対応のためでしょう。

@berryzplus
Copy link
Contributor

僕は原理主義者ではないので「正しいか」は知りませんが……

あーでもない、こーでもない、と考えても時間の無駄にするような気がします。

個人的には、事象を白か黒のいずれかに色分けして、「悪・即・斬」するのが分かりやすいと思います。
 👆こういうのを「原理主義」というのですかね。

ちなみに僕のコメント内容は「未満」ではなく「以下」で正しいです。
恐らくオリジナルのころから行間「0」は設定できます。この時点で下の行に線が引かれます。
マイナス値を許容するようになったのは、プロポーショナルフォント対応あたりからです。

自分のは、あんまし深く考えたコメントではなかったです。

なんとなく「0(=行間なし)」を指定できるのは「正しい」気がします。

設定値ゼロのときにアンダーラインが次行に被るのはマズいような気がします。
(たぶん、文字列描画矩形の外側に引いてるから次行に被るんですね。)

メイリオ的な事情で、行間に負数を指定できたほうが都合がよいこともある、は理解しました。
(メイリオ的な事情 ≒ 見やすさを考慮して行間を広めにとる設計になっている。)

みなさん優しくて、メイリオ的な事情に対して「知るかヴォケ!」とは言わない気がするので、
「行間に0以下を指定した場合にうまくいかないから」という理由で見送りになりそうな雰囲気です。

@beru beru changed the title メモリDCを利用しない場合はアンダーライン描画を行描画の直後に行う事でちらつきを抑える WIP: メモリDCを利用しない場合はアンダーライン描画を行描画の直後に行う事でちらつきを抑える Apr 6, 2021
@beru
Copy link
Contributor Author

beru commented Apr 6, 2021

変更後の動作を見ていたところ、タイプ別設定で「カーソル位置縦線」を表示させる設定をした状態で PageUp/Down 操作をすると、カーソル行より下にある縦線が消えてしまいました。
CEditView::CaretUnderLineON の中では縦線も描画しているようなので、カーソル行以降の行描画によって上書きされている気がします。

おぉ、不具合報告ありがとうございます。コメントに書いていただいた問題が起きることを確認しました。

昔も似たような不具合出したような既視感があって検索しました。おそらくhttps://github.com/sakura-editor/sakura/pull/1065#issuecomment-539565600。

@beru
Copy link
Contributor Author

beru commented Apr 6, 2021

行間を0以下にすると、カーソル行アンダーラインの位置が一つ下の行に含まれるようになるため、レンダリングの対象行が変わってしまい、PageUp/PageDownなどでアンダーラインが消えてしまうようです。

こちらの不具合報告もありがとうございます。コメントに書いていただいた問題が起きることを確認しました。

@beru beru changed the title WIP: メモリDCを利用しない場合はアンダーライン描画を行描画の直後に行う事でちらつきを抑える メモリDCを利用しない場合はアンダーライン描画を行描画の直後に行う事でちらつきを抑える Apr 6, 2021
@beru
Copy link
Contributor Author

beru commented Apr 6, 2021

行の間隔の設定値を負の値にすると行の下側が下の行に上書きされて切れてしまいますね。行単位に背景色で塗りつぶす描画を行っているからだと思いますが、テキストが切れないように重ね描きする方が奇麗な気がします。その設定に実用性があるかは置いておいて…。

@sonarcloud
Copy link

sonarcloud bot commented Apr 6, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

85.7% 85.7% Coverage
0.0% 0.0% Duplication

@AppVeyorBot
Copy link

Build sakura 1.0.3643 completed (commit d51049c9f6 by @beru)

@suconbu
Copy link
Member

suconbu commented Apr 6, 2021

ad890c4 にて、カーソル位置縦線が消える問題が解消されたことを確認しました。
また、アンダーラインについて「カーソル位置縦線が ON」または「行の間隔が 0 以下」の場合にはちらつきありで、それ以外ではちらつくことなく表示されていました。

@berryzplus berryzplus dismissed their stale review April 10, 2021 05:07

このPRで新たに発生するCodeSmellsがなくなったため。

@beru
Copy link
Contributor Author

beru commented Apr 22, 2021

レビューありがとうございます。Mergeします。
もし問題が見つかったらその時はPRを作成して修正します。

@beru beru merged commit b96e322 into sakura-editor:master Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants