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

バグ修正や小さな改善. #46

Merged
merged 3 commits into from
Feb 2, 2024
Merged

Conversation

sigma-axis
Copy link
Contributor

いくつか問題点を見つけて対処できたので提案します.

  1. DirtyCheck のバグ修正:

    1. 何か編集を加えた直後にフレーム移動するなどでタイトルを更新すると * マークが消えた.
    2. 保存をしても * マークが消えなかった.
    3. 保存したあとフレーム移動するだけで編集済み扱いになって,AviUtl を閉じようとしたときメッセージが表示されていた.

    (i) と (iii) は色々勘違いっぽいコードだったことが原因だったようです.Check() 関数は get() 関数と名前が似ているため,役割を明確にする目的で Update() 関数に改名,戻り値も void にしました.

    (ii) は手動でタイトルから * マークを消すようにしました.

  2. EditBoxTweaker の変更.

    1. DPIに合わせてフォントサイズを変える部分が実は逆効果だったので無効化.
    2. [[fallthrough]]; のコメントアウトを解除.
    3. Detours でフックする関数宣言部分を少しコンパクトに.

    (i) について,実際にテストした画像がこちらです(いずれも Segoe UI, サイズ14):

    • UIスケール100%:
      scale100

    • UIスケール125%:
      scale125

    • UIスケール150%:
      scale150

    ボックスとの比率が変わってきてしまっているのがわかると思います.いわば,比率の二重適用になっていたようでした.修正の方針としては無効化するか逆掛けするかでしょうが指定できるフォントサイズが整数に限られるため,逆掛けした場合 UIスケール > 100% のとき,違う数値を設定しても同じフォントサイズになることもあり調整しにくいと思い無効化する方針にしました.既存の設定が壊れる結果にはなりますが,以前だとスキップされていた整数値も設定できることになるのでいい面もあります.

    (ii) の[[fallthrough]] の部分は私の以前のプルリクの部分のようですが,これはその時のプロジェクトが C++14 をターゲットとしていた一方,[[fallthrough]] は C++17 以降だった名残です.今のプロジェクトは C++20 のようなのであったほうがいいです.自分で書いた部分なので自分で戻すことにしました.

    (iii) はどちらかというと個人的な趣向です.他の Detours に適用する構造体と型も統一できていると思います.

  3. Nest の修正.

    1. いくつかの場面でショートカットキーの入力が無視されていたのを修正.
    2. フロート状態のサブウィンドウを,タイトル部分を右クリックで名前を変更できるように.
    3. ペインのボーダーの「左上原点 / 右上原点」を切り替えたとき,ボーダーの画面上の位置が変わってしまっていたのを修正.
    4. ついでにフロート状態のサブウィンドウにアイコンを設定.

    (i) について.Nest の MainWindow 以外のウィンドウにフォーカスがある状態で,例えばオブジェクトの長さを変えるダイアログを出してキャンセルした直後は一度マウス操作に戻ってフォーカスを切り替えなおすなどしないとショートカットキーを受け付けませんでした.他にもフロート状態のサブウィンドウのタイトル部分をクリックした直後などにも受け付けませんでした.当該ウィンドウのメッセージハンドラにキー入力メッセージをAviUtlのメインウィンドウに丸投げする処理を書いておきました.

    (ii) について,私は AviUtl を1枚のウィンドウに収めるのではなく,いくつかのグループにドッキングして使いたい派なのですが,その時にサブウィンドウの名前が一度何かにドッキングしないと変更できないのが不便に感じたので,タイトルの右クリックメニューに項目を追加しました.

    (iii) について,レイアウトを構築していくとき不便だと感じていたので修正しました.

    (iv) はどちらかというと (ii) の副産物です.SubWindow に右クリックメニューを追加するタイミングに init() 関数を定義したのですが,MainWindowinit() 関数を参考にしたついでです.デフォルトのアイコンよりもちょっときれいになったと思います.

  4. その他小さな修正.

    1. WinUtility.hisAncestor() 関数は Win32 API の IsChild() 関数を使うほうが手短になると思います.
    2. Window.hsubclassProc() 関数に明らかに無駄な関数呼び出しがあったので無効化しました.おそらくはデバッグ用コードのコメントアウト忘れだと思いますが,呼び出される頻度の高い部分なので無効化したほうがいいと思いました.

以上です.ご確認していただければと思います.

@hebiiro hebiiro merged commit adbce63 into hebiiro:master Feb 2, 2024
@sigma-axis
Copy link
Contributor Author

sigma-axis commented Feb 2, 2024

追加で修正です.(今マージされましたが.)

  1. 特定の条件下でペインを最大化するとタブコントロールが正しく描画されなかったのを修正.

    次のような状態で,タブコントロール → サブウィンドウの順に最大化すると描画が壊れてました.

    tab_bug1

    tab_bug2

    tab_bug3

    こちらが想定する描画結果です.

    tab_bug4

    ペインのレイアウト計算時に再描画を抑制,最後に最大化ペインだけ再レイアウトして描画させる手筈が,タブコントロールだけはサイズが変化していないので再描画がされていないのが原因のようでした.然るべき条件下で InvalidateRect() を呼び出すようにしました.

  2. サブウィンドウに入れたシャトルのタイトルをクリックしたときに,そのシャトルではなくサブウィンドウにフォーカスが移っていたのを修正.

    タイトルクリックを検知して,WM_LBUTTONDOWN でのメッセージに return 0 するようにしました.

  3. DirtyCheck でのフック時に SetWindowLongPtr() でメッセージハンドラを上書きするのではなく SetWindowSubclass() を使うように変更.こちらのほうが競合が少なくなるようにと導入された関数なので機械のあるうちに更新したほうがいいと思いました.

以上です.マージありがとうございます.

@hebiiro hebiiro self-requested a review February 2, 2024 23:14
@hebiiro hebiiro added 🐛問題 🐛この項目には問題(バグ)が含まれています。 🩹 解決策 🩹この項目には解決策が含まれています。 🏷️『ワークスペース化』 🏷️ この項目は『ワークスペース化』アドインに関連しています。 🏷️『エディットボックス微調整』 🏷️この項目は『エディットボックス微調整』アドインに関連しています。 🎨リッチ 🎨この項目にはテキスト以外の情報も含まれています。 ✅完了 ✅この項目は完了しています。 labels Feb 2, 2024
hebiiro added a commit that referenced this pull request Feb 4, 2024
♻️ AviUtlウィンドウをTools::Windowでサブクラス化するように変更しました
@hebiiro hebiiro added 🏷️『終了確認』 🏷️この項目は『終了確認』アドインに関連しています。 and removed ✅完了 ✅この項目は完了しています。 labels Feb 4, 2024
hebiiro added a commit that referenced this pull request Feb 6, 2024
🔥 シャトルをクリックしたときフォーカスを当てる処理を削除しました
✨ コンテナの一部メッセージをAviUtlウィンドウに転送するようにしました
✨ サブウィンドウの一部メッセージをAviUtlウィンドウに転送するようにしました
@sigma-axis
Copy link
Contributor Author

今現在のソースをコンパイルして動作確認してみたところ,この PR での提案部分は全て正常に動いているようでした.大量の変更箇所をきちんと確認し,1つ1つ丁寧に対応して下さりありがとうございました.

あと私の反省点として,今回のこの PR は1つの案件としては変更箇所が多すぎたかもしれません.小分けして余り多数の PR を投下するのも気が引けたのと,各変更部分が conflict する部分もあったので,まとめて提案しようと判断した結果です.どの修正項目がどのコード変更部分と対応してるのか判別がし辛かったかもしれません.

もしまた PR する機会があったなら,やはり余り変更箇所が多くならないよう小分けにした方が都合がいいでしょうか?それとも今回のようなサイズでも問題なかったでしょうか?

@hebiiro
Copy link
Owner

hebiiro commented Feb 10, 2024

PR先がすべてを採用するとは限らないと思うので、少なくともコミットは小分けにしたほうがいいと思います。
あとはなるべく変更位置の前後の書式に合わせたほうがいいです。

@sigma-axis
Copy link
Contributor Author

了解しました.これからはコミットごとに変更内容をまとめるようにしてみます.改めて今回は本当にありがとうございました.

@hebiiro hebiiro added the ✅完了 ✅この項目は完了しています。 label Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✅完了 ✅この項目は完了しています。 🎨リッチ 🎨この項目にはテキスト以外の情報も含まれています。 🏷️『エディットボックス微調整』 🏷️この項目は『エディットボックス微調整』アドインに関連しています。 🏷️『ワークスペース化』 🏷️ この項目は『ワークスペース化』アドインに関連しています。 🏷️『終了確認』 🏷️この項目は『終了確認』アドインに関連しています。 🐛問題 🐛この項目には問題(バグ)が含まれています。 🩹 解決策 🩹この項目には解決策が含まれています。
Projects
Development

Successfully merging this pull request may close these issues.

2 participants