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

等幅フォント使用時に全角文字の文字幅が半角文字のちょうど2倍になるように強制的に設定 #1912

Merged
merged 8 commits into from
Feb 15, 2023

Conversation

beru
Copy link
Contributor

@beru beru commented Feb 12, 2023

PR対象

  • アプリ(サクラエディタ本体)

カテゴリ

  • 不具合修正

PR の背景

仕様・動作説明

等幅フォント使用時に全角文字の文字幅が半角文字のちょうど2倍になるように強制的に設定を行うように変更しています。

CCharWidthCache::QueryPixelWidth メソッドで GetTextExtentPoint32 関数で求める方法だとぴったしちょうど2倍にならない事への対策です。

PR の影響範囲

文字描画に影響

テスト内容

ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz
あいうえおかきくけこさしすせそたちつてとなにぬねのは

という文字列を使った場合に、等幅フォント使用時に1行目と2行目の幅が一致するかどうかを確認。

  • UD デジタル 教科書体 N-R
  • MS ゴシック
  • BIZ UDゴシック

プロポーショナルフォントでは1行目と2行目の幅が一致しない事も確認

  • MS UI Gothic
  • MS Pゴシック
  • メイリオ
  • 游ゴシック

カツオオヤツヨ
キョウノオヤツハワカメサラダヨ

という半角カタカナ文字列が半角幅で表示される事を確認

関連 issue, PR

#1250

参考資料

https://ja.osdn.net/projects/sakura-editor/forums/34071/48041/

@beru beru added the 🐛bug🦋 ■バグ修正(Something isn't working) label Feb 12, 2023
@AppVeyorBot
Copy link

Build sakura 1.0.4277 completed (commit 3ad61b747e by @beru)

@kengoide
Copy link
Member

WCODE::IsHankaku と判定条件が異なるのが気がかりです。

//0x7F ~ 0xA0 も半角とみなす
//http://ja.wikipedia.org/wiki/Unicode%E4%B8%80%E8%A6%A7_0000-0FFF を見て、なんとなく
if(wc>=0x007F && wc<=0x00A0)return true; // Control Code ISO/IEC 6429

「なんとなく」が理由とはいえ現状こういうことらしく、ここの範囲内の制御コードを表示するときに描画やキャレットの動作が破綻しないかどうかが気になります。

@AppVeyorBot
Copy link

Build sakura 1.0.4278 completed (commit 41e18682b3 by @beru)

@AppVeyorBot
Copy link

Build sakura 1.0.4279 completed (commit 740d63e2c1 by @beru)

@beru
Copy link
Contributor Author

beru commented Feb 12, 2023

@kengoide さん、ご指摘ありがとうございます。 0c304cd で判定条件を揃えるように対策を入れました。

@AppVeyorBot
Copy link

Build sakura 1.0.4280 completed (commit 7266a95ee1 by @beru)

@AppVeyorBot
Copy link

Build sakura 1.0.4281 completed (commit 0955236b2a by @beru)

@AppVeyorBot
Copy link

Build sakura 1.0.4282 completed (commit cc57442fb1 by @beru)

// 等幅フォントでも GetTextExtentPoint32 で取得したピクセル幅が半角と全角でぴったし2倍の違いにならない事がある
// 対策として半角より少しでも幅が広い場合は半角幅の2倍に揃える
if ((m_lf.lfPitchAndFamily & FIXED_PITCH) && size.cx > m_han_size.cx)
size.cx = 2 * m_han_size.cx;
Copy link
Contributor

Choose a reason for hiding this comment

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

  • 処理が論理的に誤っている(実害もある)
  • 処理が論理的に誤っている(実害はない)

どっちかな、が気になりました。

「特定の等幅フォントの特定の全角文字が見切れる」と言ってますよね?
どのフォントのどの文字でしょう。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

主に確認に使用したのは、 https://ja.osdn.net/projects/sakura-editor/forums/34071/48041/ に記載されている

  • フォント : UD デジタル 教科書体 N-R
  • フォントサイズ : 14

の組み合わせです。

CCharWidthCache::QueryPixelWidth メソッドにおいて GetTextExtentPoint32 の呼び出しで取得できる値が

  • ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz の文字の場合 : 19
  • あいうえおかきくけこさしすせそたちつてとなにぬねのは の文字の場合 : 37

となっていて、全角文字の場合に半角文字のぴったり2倍の値が取れていない事を確認しました。なおディスプレイ設定の表示スケールは 200% の環境で確認をしています。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • 処理が論理的に誤っている(実害もある)
  • 処理が論理的に誤っている(実害はない)
    どっちかな、が気になりました。

実害はあるので不具合です。

フォントとサイズによって意図した通りの幅が取れないのは、ベクトルフォントをラスタライズしてビットマップを作った際にどの整数値になるかというのが丸めの関係で変わるのかもしれないですね。

例えば、18.6 を四捨五入した整数値は 19 ですが、18.6 * 2 = 37.2 なので四捨五入した整数値は 37 になります。整数値にするとちょうど2倍にならないのはこういう事が原因なのかなと推測しています。

私は自分でFreeTypeのようにベクトルフォントをラスタライズする実装をしているわけじゃないので、間違った事を言っているかもしれません。

Copy link
Contributor

Choose a reason for hiding this comment

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

試せていないのですが、 理屈は分かりました。

  • 半角文字 19pixel -> 20文字で380pixel
  • 全角文字 37pixel -> 10文字で370pixel

同じ幅になる仕様なのに、同じにならない。

↓この画像は変更前。
image

Copy link
Contributor

Choose a reason for hiding this comment

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

↓このPR適用後。
image

kengoide
kengoide previously approved these changes Feb 15, 2023
Copy link
Member

@kengoide kengoide left a comment

Choose a reason for hiding this comment

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

様々な環境で検証しなければ修正の妥当性が判断しづらい問題かもしれません。少なくともコードについては妥当だと思いますので一度マージしてみませんか?

@AppVeyorBot
Copy link

Build sakura 1.0.4283 failed (commit 5da5ca45a3 by @berryzplus)

@berryzplus
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@berryzplus
Copy link
Contributor

CIでのテストが失敗するので諦めます。

@sonarcloud
Copy link

sonarcloud bot commented Feb 15, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

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

50.0% 50.0% Coverage
0.0% 0.0% Duplication

@AppVeyorBot
Copy link

Build sakura 1.0.4284 failed (commit d5dea1253b by @berryzplus)

@AppVeyorBot
Copy link

@beru
Copy link
Contributor Author

beru commented Feb 15, 2023

レビューありがとうございます。Mergeします。
もしこのMRの変更による問題が確認された場合は、修正用の別MRを作成して対応したいと思います。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛bug🦋 ■バグ修正(Something isn't working)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants