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

DEBUG_SETPIXELを無効化する #578

Conversation

berryzplus
Copy link
Contributor

@berryzplus berryzplus commented Oct 21, 2018

issue #568 「バージョン情報」のURL部分の1文字目より前の左上にドットがある を参照。

DEBUG_SETPIXELはvista以降で描画のデバッグをする際に発生した不具合の対策関数。
点を打つ命令(実際には2pxの線を引いている)を発行することで描画を即時反映させる目的のもの。
この関数の利用箇所は4箇所あるが、うち2箇所はデッドコード(ANSI版関数)。

aliveな関数呼出しが塗りつぶし処理の後にあるため、
描画結果にゴミが見えてしまっているバグの対策として、
点を打つ処理と塗りつぶし処理の呼出し順を入れ替える対応を行う。

追記:
現行windowsではDEBUG_SETPIXELの効果がまったくないことが判明。
ただ無駄に点を打つ処理と化しているので無効化するように修正します。

@berryzplus berryzplus mentioned this pull request Oct 21, 2018
@berryzplus berryzplus changed the title DEBUG_SETPIXELの呼出順を変える [WIP] DEBUG_SETPIXELの呼出順を変える Oct 21, 2018
@m-tmatma
Copy link
Member

https://ci.appveyor.com/project/sakuraeditor/sakura/builds/19670963
で確認しましたが、直ってないように見えます。

ダイアログ表示直後

init

マウスカーソルをリンクに合わせた後

hover

@berryzplus
Copy link
Contributor Author

点が出る事象が改善されなかったので一旦考え直します 😭

@berryzplus berryzplus force-pushed the feature/fix_debug_ext_text_out branch from 324c671 to 3bea2ec Compare October 21, 2018 13:56
issue sakura-editor#568 「バージョン情報」のURL部分の1文字目より前の左上にドットがある を参照。

DEBUG_SETPIXELはvista以降で描画のデバッグをする際に発生した不具合の対策関数。
点を打つ命令(実際には2pxの線を引いている)を発行することで描画を即時反映させる目的のもの。
この関数の利用箇所は4箇所あるが、うち2箇所はデッドコード(ANSI版関数)。

windows8.1/10ではSetPixcelしても即時更新されない模様。
紛らわしいのでコメントとSetPixelの呼出しを削っておく。
@berryzplus berryzplus changed the title [WIP] DEBUG_SETPIXELの呼出順を変える DEBUG_SETPIXELを無効化する Oct 21, 2018
@ds14050
Copy link
Contributor

ds14050 commented Oct 23, 2018

Windows Vista でのデバッグに差し支えるらしいので、単純に SetPixel を消したのでは自分が LGTM を出すことはありません。不都合なことでも不具合でもありませんので。

@berryzplus
Copy link
Contributor Author

Windows Vista でのデバッグに差し支えるらしいので、単純に SetPixel を消したのでは自分が LGTM を出すことはありません。不都合なことでも不具合でもありませんので。

vistaでのデバッグで支障があったかどうかはもはや関係ありません。
何故ならvs2017はvistaでは動作しないからです。 https://docs.microsoft.com/ja-jp/visualstudio/productinfo/vs2017-system-requirements-vs

不都合でないかどうかはこのissueに納得のいく回答を出せるかで判断したいです。

issue #568 「バージョン情報」のURL部分の1文字目より前の左上にドットがある を参照。

当初、ぼくも「不都合」ではない、と考えていました。
しかし、SetPixelで点を打つ処理は、デバッグの挙動に何の効果も与えないことが判明しました。
「デバッグのために必要」だから「不都合」ではない、という考えは崩れました。

何の意味もなく余分な点を打つのは、プログラムの不具合ではないんでしょうか?

@ds14050
Copy link
Contributor

ds14050 commented Oct 28, 2018

vistaでのデバッグで支障があったかどうかはもはや関係ありません。
何故ならvs2017はvistaでは動作しないからです。

プロセスにアタッチしてデバッグするくらいのことは VS2017 に限らずできるのではありませんか?

@berryzplus
Copy link
Contributor Author

プロセスにアタッチしてデバッグするくらいのことは VS2017 に限らずできるのではありませんか?

起動済みプロセスへのアタッチは、デバッガならば備えてる機能だと思います。
vs2017じゃなくてもデバッグはできますね。

それはその通り・・・あれ?。

そもそもの話、vistaは稼働環境としても対象外です。

ちょっと待った。

元の開発者がなんで DEBUG_SETPIXEL を作ったか(=vista環境で何に困ったか)を解き明かさないと先に進まない気がしてきました。

なんとなく予想しているところでは Display Window Manager の導入により、描画命令転送のスタイルが変わったことによる変化が原因なんではないかと思っています。

DWMが導入されて以降、ウインドウは3D空間内に配置されたポリゴンとして表現されるようになりました。アプリがDirect3Dに対応しているかどうかに関係なく、すべてのアプリは2Dサーフィス上に構築された平面を描画するように変わりました。ポリゴンのサーフィスに描画するってことはつまり、テクスチャビットマップに絵を描いてるってことです。従来であれば描画命令はGDI命令に変換されていましたが、vistaからは描画命令はDirect3D命令に変換されるようになったわけです。

予想ですがおそらく、vistaより前の描画コードはもっとステップバイステップな感じだったんだと思います。(ちょっとずつ、ちょっとずつ描いていく感じだったという意味です。)だからこそvistaになって描画スタイルが変わって困ったんじゃないかな、と思うわけです。現状のデバッグで困らなくなったのは、ステップバイステップな部分が徐々に関数化されたからなんじゃないかな、と思うわけです。

DirectXの描画は、バックエンドサーフィス上に書き込んだ完成図をフロントエンドサーフィスに一括転送するやり方が基本です。つまり、途中経過は見られません。どうしても途中経過を見たい場合は、描画トランザクションを1描画命令ごとに終了させる必要があると思います。

もともと入っていた DEBUG_SETPIXEL の実装は、実行した描画の後に描画領域外に点を打つ処理でした。vistaでの挙動は確認していませんが、win8.1とwin10では描画は更新されず、描画トランザクション終了後に結果が反映されていました。描画領域外にはちゃんと点が打たれていました。

もしかすると、vistaでは描画領域外に点を打つ処理が描画トランザクションを終了させるのかも知れません。実際の処理で描画終了後に何か処理があるわけではないので、描画トランザクションを強制終了させる意味はないように感じられますが、効果があった可能性はゼロじゃないです。

思い当たることを探って色々書いてみました。

なんか結局、もともと意味のあるコードじゃなかったんじゃない?ってのが感想です。
当時といまの違いは、描画領域外に点が打たれるか打たれないかの違いのような・・・。

@ds14050
Copy link
Contributor

ds14050 commented Oct 28, 2018

もしかすると、vistaでは描画領域外に点を打つ処理が描画トランザクションを終了させるのかも知れません。実際の処理で描画終了後に何か処理があるわけではないので、描画トランザクションを強制終了させる意味はないように感じられますが、効果があった可能性はゼロじゃないです。

トランザクションの強制終了によりステップバイステップで描画結果がデスクトップに反映されたのではないでしょうか。berryzplus さんの文章を読んでの感想ですが。


元からそういう考えではあったのですが、Vista より新しい Windows では効果が見られない不十分なデバッグ用の手段であることから、他の人の承認の元にコードを取り除くことに反対しているわけではありません。困るのが自分一人なら自分が困ったときに対処を考えれば済むことですので。

@berryzplus
Copy link
Contributor Author

もうしばらく考えたいので、PRはこのままにしておきます。

@KENCHjp KENCHjp added the CI appveyor など CI 関連 【ChangeLog除外】 label Dec 5, 2018
@beru beru removed the CI appveyor など CI 関連 【ChangeLog除外】 label Dec 28, 2018
@berryzplus
Copy link
Contributor Author

この対応って何だっけ?と思って最新版ビルド 1558で確認したら対処したかった現象が発生しなくなっていました。(フォーカス時に黒点が打たれる

同じ問題が再発したら同様のPRを投げる可能性はありますが、今となっては対処不要のようなのでこのPRはクローズにします。

@berryzplus berryzplus closed this Jan 12, 2019
@berryzplus berryzplus deleted the feature/fix_debug_ext_text_out branch January 12, 2019 13:14
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.

5 participants