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

ダイアログフォント美化 #1421

Merged
merged 16 commits into from
Oct 11, 2020

Conversation

suconbu
Copy link
Member

@suconbu suconbu commented Oct 3, 2020

PR の目的

すべてのダイアログボックスで Windows のシステムフォントを使うようにし文字の見た目を良くします。

※システムフォントではなくエディタと同じフォントの方が適当と思われる部分 (キーワード補完やアウトライン解析など) があります。この点は別の PR で対応したいと考えています。

例:
-|-
-|-
現状 | image
変更後 | image

カテゴリ

  • 仕様変更

PR の背景

PR のメリット

ギザギザしていない、きれいな文字が表示されるようになります。
(Windows7 以前ではユーザーが選択しているフォントに依存)

MS Pゴシックが苦手な人も気持ちよく使えるようになるかもしれません。

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

特にありません。

仕様・動作説明

ダイアログボックスのフォントが「MS Pゴシック」からシステムフォント (下表) に変わります。

OS 標準のシステムフォント
Windows10 Yu Gothic UI
Windows8 Meiryo UI
Windows7/Vista: メイリオ (メッセージボックス)

テスト内容

  • システムフォントに「Yu Gothic UI」が設定されている状態 (Windows10 標準状態) で、すべてのダイアログボックスを表示させ、文字切れや文字重なりがないことを目視確認します。
  • フォント変更ツール (参考資料) にてシステムフォントに「Meiryo UI」「メイリオ」を設定し、それぞれ「印刷ページ設定」ダイアログにて文字切れや文字重なりがないことを目視確認します。
    →全部確認するのは大変なのでサイズ調整前に一番文字切れが目立っていたものだけとしました。

PR の影響範囲

関連 issue, PR

GREPダイアログ内の全コンボボックスに対してフォント設定で指定されたフォントを使用する #1400

参考資料

@suconbu
Copy link
Member Author

suconbu commented Oct 3, 2020

CDialogを継承していないものたちを引き続き対応中です。

@AppVeyorBot
Copy link

Build sakura 1.0.3129 completed (commit d20e914a8a by @suconbu)

Copy link
Contributor

@beru beru left a comment

Choose a reason for hiding this comment

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

英語版のリソースファイル sakura_lang_en_US/sakura_lang_rc.rc も調整が必要かもしれません。
ただこのPRで一挙にやらないでも良いとは思います。

@beru beru added the enhancement ■機能追加 label Oct 3, 2020
@berryzplus
Copy link
Contributor

berryzplus commented Oct 3, 2020

ダイアログリソースのコントロールサイズの指定は、ダイアログ単位というダイアログ作成時に指定したフォントの平均文字幅と平均文字高さから算出される値を基準に書くものなので、フォントを変えるとめちゃめちゃたくさん変更しないといけないんですよ~~~という基本の話題はみなさん分かってるものとしてスルーしときます。(してない。

ツールでもって自動補正させたものを目視でチェックするって方向ですね。いいと思います。

ただ、ダイアログリソースのプロパティで DS_SHELLFONT を指定すると、システムフォントに対応させることができる という技術ネタがあるのでcppソース側の変更は要らんかも知れないです。

https://docs.microsoft.com/en-us/windows/win32/dlgbox/dialog-box-styles
https://dev.activebasic.com/egtra/2014/06/16/666/

ここの STYLE に DS_SHELLFONT を指定するです。

IDD_ABOUT DIALOG DISCARDABLE 0, 0, 245, 228
STYLE DS_MODALFRAME | DS_CENTER | WS_POPUP | WS_CAPTION | WS_SYSMENU
CAPTION "バージョン情報"
FONT 9, "MS Pゴシック"

何故か「文字コードの指定」ダイアログだけ何故かDS_SETFONTが指定されていました。

IDD_SETCHARSET DIALOGEX 0, 0, 186, 45
STYLE DS_CENTER | DS_MODALFRAME | DS_SETFONT | WS_BORDER | WS_CAPTION | WS_DLGFRAME | WS_POPUP
CAPTION "文字コードの指定"
FONT 9, "MS Pゴシック"

@suconbu
Copy link
Member Author

suconbu commented Oct 4, 2020

@beru さん

英語リソースの方は現状で Tahoma フォントが指定されていてきれいでしたので、こちらは現状維持となるよう対応したいと思います。
※仮で英語も Yu Gothic UI にしてみたところでは調整の難しそうな横方向のはみ出しが結構出てしまい、変更してしまうとかなり大変そうな雰囲気でした。

ただ、英語はタブだけがなぜかギザギザなので、こちらは理由を調べてみて軽微修正であれば直しておきたいです。

@berryzplus さん

ダイアログ単位:水平方向は平均文字幅の 1/4、垂直方向は文字高の 1/8 ということから、おそらく現状のテキスト高さには 8 が設定されている箇所が多かったようですが、フォント変更後は以下のような文字切れなどが見られたため各部調整必要となりました。

image

ダイアログリソースのプロパティで DS_SHELLFONT を指定すると、システムフォントに対応させることができる

情報ありがとうございます。
しかし、結果は以下の通りでうまくいかずでした。指定できたら便利そうなだけに残念。

現状 (MS Pゴシック) DS_SHELLFONT + MS Pゴシック DS_SHELLFONT + MS Shell Dlg
image image image

以下の記事では「Windows 2000 default shell font」とあるので、MS UI Gothic + Tahoma が適用されてしまうのかもしれませんね。
https://devblogs.microsoft.com/oldnewthing/20050204-00/?p=36523

@suconbu
Copy link
Member Author

suconbu commented Oct 4, 2020

フォント変更後は以下のような文字切れなどが見られた

リソースで指定されたフォント (= MS Pゴシック) で配置が決定され、実行時に設定されたフォントの大きさは考慮されないことに気づきました。なので↑となるのは当然でした。反映がされるものと誤解をしていました。

リソースに予め Yu Gothic UI を設定しておけば現状より余裕を持った配置となってくれるので、文字重なり予防の点でこの方が良いかもしれません。少し考えてみます。

@suconbu
Copy link
Member Author

suconbu commented Oct 4, 2020

リソース側で Yu Gothic UI を設定した場合は全体的なサイズ感が想定以上に変わってしまいました (右端) ので、
今回の PR としては、基本は現状のサイズ/配置のまま問題がある部分について個別調整していく (中央)、で進めようと思います。

現状 (変更前) 実行時 Yu GothicUI 設定 (+調整) リソースで Yu Gothic UI 指定
image image image

@beru
Copy link
Contributor

beru commented Oct 4, 2020

リソース側で Yu Gothic UI を設定した場合は全体的なサイズ感が想定以上に変わってしまいました (右端) ので、
今回の PR としては、基本は現状のサイズ/配置のまま問題がある部分について個別調整していく (中央)、で進めようと思います。

おぉ、右端のは結構縦長ですね。

https://forest.watch.impress.co.jp/docs/shseri/rewin10/1011993.html

Yu Gothic UI は縦長にデザインされた書体という事を今まで知りませんでした。。

ユーザーの好みに合わせて設定画面で選択できるようにするのが良いんでしょうか?ただリソースダイアログに設定したフォントによってダイアログ内のコントロールの最適な位置やサイズが変わるとなると、実行時に設定する方法でも課題は残りそうですね。。

@AppVeyorBot
Copy link

Build sakura 1.0.3138 completed (commit 029aaf8fa3 by @suconbu)

@suconbu
Copy link
Member Author

suconbu commented Oct 4, 2020

ユーザーの好みに合わせて設定画面で選択できるようにするのが良いんでしょうか?ただリソースダイアログに設定したフォントによってダイアログ内のコントロールの最適な位置やサイズが変わるとなると、実行時に設定する方法でも課題は残りそうですね。。

ダイアログのフォントについてはユーザーが自由に選べるようにするよりは、システムに合わせる (タイトルバーやメニュー等と統一する) の方が良いかなと個人的には考えていました。
HTML などのような動的に再配置する仕組みがない中、実行時にフォント設定するとなるとレイアウトの問題がおきるリスクは避けられない気がしますので、できるだけ自由度は少ない方向が安全というところですね。

幸い (?) Windows8 以降ではユーザーが気軽にフォント変更できないようになりましたので、各 OS バージョンでの標準フォントにて目視チェックしておけば大丈夫なはず、という目論見でいます。

……とはいえ、MS Pゴシック基準で配置されたものにサイズ感の違うフォントが載る点、気持ち悪さは残る気がしますので、いつか Yu Gothic UI などを基準として全体的に再配置すべきかもしれません。

@suconbu
Copy link
Member Author

suconbu commented Oct 4, 2020

英語はタブだけがなぜかギザギザなので、こちらは理由を調べてみて軽微修正であれば直しておきたいです。

こちらはまだ原因特定できておらず、おそらく今回は見送りとなりそうです。

@beru
Copy link
Contributor

beru commented Oct 4, 2020

動かしてみましたが、特に問題無さそうですね。文字が前より少し細って見えますが綺麗になったとおもいます。

@berryzplus
Copy link
Contributor

……とはいえ、MS Pゴシック基準で配置されたものにサイズ感の違うフォントが載る点、気持ち悪さは残る気がしますので、いつか Yu Gothic UI などを基準として全体的に再配置すべきかもしれません。

変えるなら FONT 9, "MS Pゴシック" を変えたいなぁ、とわがまま言ってみます。

vs2017 の最新版で新規ダイアログリソースを追加するとこうなるです。

IDD_DIALOG1 DIALOGEX 0, 0, 311, 177
STYLE DS_SETFONT | DS_MODALFRAME | DS_FIXEDSYS | WS_POPUP | WS_CAPTION | WS_SYSMENU
CAPTION "Dialog"
FONT 8, "MS Shell Dlg", 400, 0, 0x1
BEGIN
    DEFPUSHBUTTON   "OK",IDOK,200,156,50,14
    PUSHBUTTON      "キャンセル",IDCANCEL,254,156,50,14
END

DS_SETFONT | DS_FIXEDSYSDS_SHELLFONT と同じ意味です。
https://devblogs.microsoft.com/oldnewthing/20050207-00/?p=36513

DS_SHELLFONT + MS Shell Dlg を指定したときの効果は、システムのUIフォントが使われる、です。
古い文献には MS UI Gothic になると書いてあるものもありますが、xpのUIフォントはそれなんで合ってます。

気が向いたらそちらのアプローチでPRおこしてみようかな、と思います。

@berryzplus
Copy link
Contributor

DS_SHELLFONT + MS Shell Dlg を指定したときの効果は、システムのUIフォントが使われる

何故だ!そんな挙動にはならん!w

@suconbu
Copy link
Member Author

suconbu commented Oct 5, 2020

何故だ!そんな挙動にはならん!w

タイトルバー等と同じフォントにならないですよね。。
「MS Shell Dlg」は実際のシステムフォントとは関係なしに XP, 7, Vista (おそらく 8, 10 でも?) の日本語環境では MS UI Gothic に割り当てられてしまうようです。
https://docs.microsoft.com/en-us/windows/win32/intl/using-ms-shell-dlg-and-ms-shell-dlg-2

@suconbu
Copy link
Member Author

suconbu commented Oct 5, 2020

動作確認中、ファイル->印刷ページ設定 を開いて閉じるとフォントがおかしくなる現象を見つけたため修正中です。
キャッシュした共有のフォントハンドルが CDlgPrintSetting::OnDestroy で破棄されてしまっている気がします。

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.

気付いたことコメントしてみました。

if( !SystemParametersInfo( SPI_GETNONCLIENTMETRICS, 0, &metrics, 0 ) ) {
return NULL;
}
LOGFONT lfFont = metrics.lfMessageFont;
Copy link
Contributor

Choose a reason for hiding this comment

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

技術的には、この方式で Final Answer ですね・・・。

sakura_core/util/window.cpp Outdated Show resolved Hide resolved

NONCLIENTMETRICS metrics = {};
metrics.cbSize = sizeof( metrics );
if( !SystemParametersInfo( SPI_GETNONCLIENTMETRICS, 0, &metrics, 0 ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

この関数にはマルチDPI対応版が存在するので、そちらを使うのがよいと思います。

Copy link
Member Author

Choose a reason for hiding this comment

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

SystemParametersInfoForDpiを使うよう修正します。

Copy link
Member Author

Choose a reason for hiding this comment

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

コンパイル通らないと思ったら、利用できるのがターゲットが Windows10 以降からのようでした。

#if(WINVER >= 0x0605)  // 0x0601
WINUSERAPI
_Success_(return != FALSE)
BOOL
WINAPI
SystemParametersInfoForDpi(

Copy link
Contributor

Choose a reason for hiding this comment

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

一旦保留して ForDPI なし版でいきますか・・・

#if(WINVER >= 0x0605)  // 0x0601

ただ、0x0605ってバージョンは謎っすね。
https://docs.microsoft.com/en-us/cpp/porting/modifying-winver-and-win32-winnt?view=vs-2019

マニュアルによると win 10 1607 以降 only ってことらしいです。
https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-systemparametersinfofordpi

@@ -310,3 +310,61 @@ void CFontAutoDeleter::Release()
}
}
#endif

HFONT GetSystemFont( LONG nHeight )
Copy link
Contributor

Choose a reason for hiding this comment

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

windowsがマルチDPIに対応しているので、ラップする関数を作るならマルチDPI対応の検討をしとくべきだと思います。旧バッチの適用なら仕方ないですが、新規作成ならマルチDPI対応を作ったうえで固定DPI対応版を用意するか検討したいっす。

Copy link
Member Author

Choose a reason for hiding this comment

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

この関数についてマルチDPI対応版/固定DPI対応版のイメージがつきませんでした。
現状のnHeightは論理ピクセル単位ですが、これとは別に物理ピクセル単位を入力できる版を用意する、というような感じでしょうか??

Copy link
Contributor

Choose a reason for hiding this comment

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

マルチDPI対応版APIが使えなくなった時点でこのコメントは意味を失いました。

LONG nHeight に気づかなかったので、UINT dpi を追加したらいいかな?と思っていました。

システムフォントにはサイズを含むような気がします。
任意の高さを指定できてしまうと別なフォントを作ってしまうようなw

「論理単位」に対応する実際の大きさを表す用語は「デバイス単位」と思われます。

GDIのマッピングモードで変えられますが、「論理単位:デバイス単位=1:1」で使う人が多いような気がします。ディスプレイのデバイス単位って、つまりピクセルですから。

Copy link
Member Author

Choose a reason for hiding this comment

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

任意の高さを指定できてしまうと別なフォントを作ってしまうようなw

確かにそうですね。あくまでシステムフォント「風」なものを提供する関数なので、関数名は再考の余地ありかもしれません。少し検討してみますがいいもの浮かばなければこのままとさせて下さい。

論理単位、デバイス単位、それと DPI の関係については以下のように理解を持ちましたが、間違っていないでしょうかね?
「論理単位」-- マッピングモード適用 --> 「デバイス単位」 -- DPI 倍率適用 --> 「ピクセル」

Copy link
Contributor

Choose a reason for hiding this comment

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

任意の高さを指定できてしまうと別なフォントを作ってしまうようなw

確かにそうですね。あくまでシステムフォント「風」なものを提供する関数なので、関数名は再考の余地ありかもしれません。少し検討してみますがいいもの浮かばなければこのままとさせて下さい。

これは システムフォント「風」 で納得です。

論理単位、デバイス単位、それと DPI の関係については以下のように理解を持ちましたが、間違っていないでしょうかね?
「論理単位」-- マッピングモード適用 --> 「デバイス単位」 -- DPI 倍率適用 --> 「ピクセル」

設計で決めた大きさ(96dpi想定のpx値) -- DPI倍率適用 -- 論理単位 -- マッピング適用 -- デバイス単位

このへんを見たらイメージ付くかも。
https://www.tokovalue.jp/function/SetMapMode.htm
アプリは通常、MM_TEXTで変換しとります。

Copy link
Member Author

Choose a reason for hiding this comment

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

すいません、ありがとうございます。分かりました。

今回だと DPI 倍率適用済みの値が入力され、そのサイズでフォント作成するので問題ないですが、
将来、システムフォントそのままのサイズが欲しい!となった時には、DPI 値を入力引数に加えつつ SystemParametersInfoForDpi を呼ぶ、呼べないなら CDPI クラス使ってサイズ変換、となりそうです。

sakura_core/util/window.cpp Outdated Show resolved Hide resolved
@beru
Copy link
Contributor

beru commented Oct 11, 2020

今になって気づいたのですが、表示スケールが 200% にもなると元の MS Pゴシック でも ClearType が適用されて綺麗に表示されるのですね。。

表示スケール 100% の場合にどうなるか確認してみました。

変更前

image

変更後

image

変更後は書体が変わってアンチエイリアシングが掛かってますね。変更後の方が行間が少し空いていて読みやすいですね。

アンチエイリアシングすると表示が必要以上にぼやけてしまって人によってはそれが気になりますが、高解像度のディスプレイで表示スケールを上げて利用する分にはあまり気にならないですね。

@berryzplus
Copy link
Contributor

berryzplus commented Oct 11, 2020

#1421 (review) のコメントの問題って「文字切れ」なんだろうか?というのがちらっと気になりました。
(解決済みなのでどうでもいい話 😃

#1424 でリソーススクリプトを変更したのにマージ後にコンフリクトが発生しないのは、この変更が「ダイアログ」を対象にしているからなのかな?とか。( #1424の対象は「プロパティーシート」なのでちょっと違う・・・)

* 履歴とお気に入りの管理のヘルプボタンとサイズ変更グリップとの重なり
* 共通設定->支援タブのmigemoとテキストボックスとの重なり
* 共通設定->編集タブの「改行コードNEL,PS,LSを有効にする」の右側文字切れ
* タイプ別設定->ウィンドウタブの「折り返し単位」の右側文字切れ
* タイプ別設定->正規表現キーワードタブのDLL名下部文字切れ
* 印刷ページ設定の「ヘッダー」付近のテキストボックス上部切れ
@suconbu suconbu force-pushed the feature/use_systemfont_in_dlg branch from b7808c0 to f4a5529 Compare October 11, 2020 12:03
@AppVeyorBot
Copy link

Build sakura 1.0.3152 completed (commit 4802ad7b66 by @suconbu)

@suconbu
Copy link
Member Author

suconbu commented Oct 11, 2020

200% の状態で他に同じような問題出ていないか点検するため一旦 Draft にします。

数か所見つかりましたので追加で修正をしました。
(指摘頂いたようながっつりと重なっているものはありませんでした)

@suconbu suconbu marked this pull request as ready for review October 11, 2020 12:25
@suconbu
Copy link
Member Author

suconbu commented Oct 11, 2020

@berryzplus さん

#1421 (review) のコメントの問題って「文字切れ」なんだろうか?というのがちらっと気になりました。

問題の部分が下にある "原作者..." のテキスト (LTEXT) に上書きされて切れているように見えていたようで、実際は「重なり」でした。
この LTEXT にちょっと動いてもらうことでうまく収まりました。

#1424 でリソーススクリプトを変更したのにマージ後にコンフリクトが発生しないのは、この変更が「ダイアログ」を対象にしているからなのかな?とか。( #1424の対象は「プロパティーシート」なのでちょっと違う・・・)

私の変更でも同じプロパティシートを編集していますが、たまたま同じ行に変更が入らなったようです。

@AppVeyorBot
Copy link

Build sakura 1.0.3153 completed (commit 746a1638a6 by @suconbu)

Copy link
Contributor

@beru beru left a comment

Choose a reason for hiding this comment

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

f4a5529 を確認しました。

共通設定のファイルの ←注:ドロップファイルは1つのみ有効 というラベルの右端が欠けて表示されているのでお手数ですが修正をお願いします。

image

@suconbu
Copy link
Member Author

suconbu commented Oct 11, 2020

共通設定のファイルの ←注:ドロップファイルは1つのみ有効 というラベルの右端が欠けて表示されている

修正しました。ここは「1つのみ有効」だったのですね笑
確認時、「1つのみ有(あり)」と読んでスルーしてしまっていました。
さらに PR 対応前は「1つのみ」となっていて、これまた違和感がなく全く気づきませんでした。

変遷:

v2.4.2.0 f4a5529 04120b3
image image image

@AppVeyorBot
Copy link

Build sakura 1.0.3154 completed (commit a803d2f89f by @suconbu)

Copy link
Contributor

@beru beru left a comment

Choose a reason for hiding this comment

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

問題ないと思います。

@beru
Copy link
Contributor

beru commented Oct 11, 2020

04120b3 の修正も確認して問題ないと思いますので Merge します。

@beru beru merged commit bb83d9d into sakura-editor:master Oct 11, 2020
@beru
Copy link
Contributor

beru commented Oct 11, 2020

f4a5529 では色々な箇所の調整がされていますね。
コミットメッセージに説明がしっかり書かれているのが良いですね。

@suconbu
Copy link
Member Author

suconbu commented Oct 11, 2020

レビュー、マージありがとうございました。

英語はタブだけがなぜかギザギザなので、

こちらは引き続き調べてみます。

@berryzplus
Copy link
Contributor

#1421 (review) のコメントの問題って「文字切れ」なんだろうか?というのがちらっと気になりました。

問題の部分が下にある "原作者..." のテキスト (LTEXT) に上書きされて切れているように見えていたようで、実際は「重なり」でした。

「文字切れ」とは、文字の一部分が欠けて表示される不具合だと思います。
まったく表示されていないように見えるのを「文字切れ」と言ってよいのか?という禅問答っす(キレテナーイ

#1424 でリソーススクリプトを変更したのにマージ後にコンフリクトが発生しないのは、この変更が「ダイアログ」を対象にしているからなのかな?とか。( #1424の対象は「プロパティーシート」なのでちょっと違う・・・)

私の変更でも同じプロパティシートを編集していますが、たまたま同じ行に変更が入らなったようです。

システムフォントでなくシステムフォント風を使っている(≒高さは変わらない)のが理由で、高さの変更は影響しなかったかもです。

@beru
Copy link
Contributor

beru commented Oct 11, 2020

#1424 でリソーススクリプトを変更したのにマージ後にコンフリクトが発生しないのは、この変更が「ダイアログ」を対象にしているからなのかな?とか。( #1424の対象は「プロパティーシート」なのでちょっと違う・・・)

私の変更でも同じプロパティシートを編集していますが、たまたま同じ行に変更が入らなったようです。

システムフォントでなくシステムフォント風を使っている(≒高さは変わらない)のが理由で、高さの変更は影響しなかったかもです。

下記の変更で高さを変えた事との関連を話しているっぽいですね。

https://github.com/sakura-editor/sakura/pull/1424/files#diff-67955f03bb0bbb169f93ff6fa4998095L1190

いやーしかしこのPRでリソーススクリプトの調整が数多く行われましたね。リソースエディタでの編集ではなくてテキストエディタで編集してビルドして動作確認しての繰り返し作業だと思うのでかなり大変だったんじゃないですか?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ■機能追加
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants