IVSの異体字セレクタに対応する #1937

berryzplus · 2023-09-23T13:11:17Z

PR対象

アプリ(サクラエディタ本体)
テストコード

カテゴリ

追加

PR の背景

異体字（IVS）への対応 #1899 を解決します。
Windows GDI自体はIVSに対応していることが分かったので、
サクラエディタ側の描画単位を変更することでIVSに対応させます。

変更前：　1文字ずつ描画。UTF16のサロゲートペアのみ2ワードを1字として描画。
変更後：　1文字ずつ描画。「正字+異体字セレクタ」は3ワードを1字として描画する仕様を追加。

IVS以外の異体字セレクタについてはPRの対象ではなりません。

仕様・動作説明

過去の「サロゲートペア対応」を参考に、「1文字ずつ描画」の機構を活かしつつIVSに対応できるよう修正します。
一言で説明すると「正字+異体字セレクタ」を1字とみなすように変更します。

現在位置の文字が何ワードで構成されるかを調べる静的メソッド CNativeW::GetSizeOfChar にコードを追加してIVS対応にします。
現在位置の次の文字位置を調べる静的メソッド CNativeW::GetCharNext にコードを追加してIVS対応にします。
現在位置の前の文字位置を調べる静的メソッド CNativeW::GetCharPrev にコードを追加してIVS対応にします。
現在位置の文字が何ワードで構成されるかを調べる静的メソッド CNativeW::GetSizeOfChar にコードを追加してIVS対応にします。
指定した文字列に対する文字幅配列を生成するメソッド CTextMetrics::GenerateDxArray にコードを追加してIVS対応にします。
文字列描画メソッド CTextDrawer::DispText に元々あったサロゲートペアの下位サロゲートのための処理を拡張して、IVSの異体字セレクタに対応させます。

PR の影響範囲

IVSの異体字セレクタで表現される文字を表示できるようになります。
文字の表示（印刷）、選択、編集に影響する変更です。
パフォーマンスを悪化させる変更です。

この対応で、とりあえず見た目IVS文字列を表示＆編集できるようになります。

テスト内容

テストファイルこれです。 ivs.txt
「葛城市」と入力して表示できること、選択できること、削除できることを確認しました。

参考資料

Wikipedia 異体字セレクタ

usagisita · 2023-09-23T13:44:45Z

単語単位の選択で、IVSも一緒に選択するようにしないとカーソル移動が変な動作になりませんか？
https://github.com/sakura-editor/sakura/blob/master/sakura_core/parse/CWordParse.cpp#L187
この辺りです。

AppVeyorBot · 2023-09-23T13:51:26Z

❌ Build sakura 1.0.4322 failed (commit 93d09eebfe by @berryzplus)

berryzplus · 2023-09-23T14:51:14Z

単語単位の選択で、IVSも一緒に選択するようにしないとカーソル移動が変な動作になりませんか？
https://github.com/sakura-editor/sakura/blob/master/sakura_core/parse/CWordParse.cpp#L187

そこの処理は単語検索とかで使う文字種判別のためのものです。

現状で、カーソル移動や単語選択に問題はなさそうなです。
このPRが目指すのは「とりあえずIVS対応してみる」で「困ったら都度対処」で良いと思ってます。

berryzplus · 2023-09-23T15:58:51Z

対応要りそうです。

sakura/sakura_core/parse/CWordParse.cpp

Lines 143 to 144 in d083650

    
           int nCharChars = CNativeW::GetSizeOfChar( pData, pDataLen, nIdx ); 
        
           if( nCharChars == 0 ){

if (nCharChars == 3) のブロックを追加しとかないとIVSが「その他文字」になってしまい、「葛城市」を単語検索できない事態になりそうです。

AppVeyorBot · 2023-09-23T17:26:18Z

❌ Build sakura 1.0.4323 failed (commit 2f8d5ea217 by @berryzplus)

AppVeyorBot · 2023-09-24T07:55:37Z

✅ Build sakura 1.0.4324 completed (commit 456cd17ef6 by @berryzplus)

AppVeyorBot · 2023-09-24T08:37:38Z

✅ Build sakura 1.0.4325 completed (commit 2558967852 by @berryzplus)

AppVeyorBot · 2023-09-24T09:11:03Z

✅ Build sakura 1.0.4326 completed (commit 567b1e6446 by @berryzplus)

tats-u · 2023-09-24T12:26:32Z

IVS付きの字の直後でバックスペースを押すと現在はまるごと消えてしまいますが、VS Codeは正字になります。

他の正しく表示・カーソル移動できるソフトはまるごと消える実装が多いようなのであくまでご参考までに。

メモ帳
Word・Excel
Windowsのタスクバー・スタートメニューの検索ボックス
Firefoxの入力コンポーネント
NeoVim
Edge・Chromeの入力コンポーネント

ただし👨‍👩‍👦‍👦（UTF-16だとなんと11文字分、32でも7文字分の化け物）のような複合絵文字は

Excel
Firefox
(NeoVim) ※正しく表示できない。UTF-32コードポイント1個分ずつのカーソル移動・削除となる

だとバックスペースを押すごとに切りの良いところまで消されます（この絵文字だと家族一人分ずつ減る挙動をします）

berryzplus · 2023-09-24T14:58:07Z

IVS付きの字の直後でバックスペースを押すと現在はまるごと消えてしまいますが、VS Codeは正字になります。

一旦、そういう仕様で実装してます。

このPRの趣旨は「正字 + 異体字セレクタを1字と扱う」なので、後方から削ったときに異体字セレクタだけ消えるのはおかしな挙動に思えます。

また、サクラエディタの内部実装の都合で削除対象文字列は「選択できる」必要があります。
異体字セレクタを単体で選択できてしまうと描画に困りそうです。
選択せずに削除するなら新しく削除関数を作る必要が出てくるのでPRが大変になりそうです。

berryzplus · 2023-09-25T01:09:36Z

GetCharNext/GetCharPrevが「正字 + 異体字セレクタ」を1字と扱うのはおかしい気がしてきました。
これらの関数には元々同名のWin32 API関数があり、オリジナルと大きく挙動を変えるのは得策でないように思います。

tats-u · 2023-09-25T02:37:29Z

一旦、そういう仕様で実装してます。

メモ帳やWord、Chromium系もそういう仕様なのでそれで問題ないと思います。

GetCharNext/GetCharPrev

「書記素クラスタ」（grapheme cluster）という概念があるので改名の参考になればと思います。

https://ufcpp.net/blog/2017/10/graphemesplitter/

例：GetPreviousGraphemeHead

今回は自前実装でいいですが、最終的にはICUを使うことになります。ICUはWindows 10以降に標準付属しています。
（将来用に、今のうちに4個以上のUTF-16コードポイントが1個の表示・削除・カーソル移動単位になることに耐えれるような実装にしておけるのが理想形です）

https://scrapbox.io/bbr-program-memo/Unicode%E3%81%AE%E6%96%87%E5%AD%97%E6%95%B0%E3%82%92%E6%9B%B8%E8%A8%98%E7%B4%A0%E5%8D%98%E4%BD%8D%E3%81%A7%E6%95%B0%E3%81%88%E3%82%8B

https://learn.microsoft.com/ja-jp/windows/win32/intl/international-components-for-unicode--icu-

sonarcloud · 2023-09-25T16:49:12Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

97.6% Coverage
0.0% Duplication

The version of Java (11.0.20) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

AppVeyorBot · 2023-09-25T17:05:54Z

✅ Build sakura 1.0.4327 completed (commit 47d9a2fe04 by @berryzplus)

berryzplus · 2023-09-28T13:56:17Z

sakura_core/view/CEditView_Command_New.cpp

@@ -613,7 +613,7 @@ void CEditView::DeleteData(
 			nNxtPos = GetCaret().GetCaretLayoutPos().GetX() + CLayoutInt(pcLayout->GetLayoutEol().GetLen()>0?1+m_pcEditDoc->m_cLayoutMgr.GetCharSpacing():0);
 		}
 		else{
-			nNxtIdx = CLogicInt(CNativeW::GetCharNext( pLine, nLineLen, &pLine[nCurIdx] ) - pLine);
+			nNxtIdx = nCurIdx + CNativeW::GetSizeOfChar( pLine, nLineLen, nCurIdx);


この行に対して単体テストを実行できないのが気持ち悪いです。

この行は以下の場合に呼ばれます。

Deleteキー押下時、選択範囲なしの場合に現在位置の1文字を削除する。

変更前：現在位置の1コードポイントを削除。
変更後：現在位置の1字を削除。

アサーションを書けないのにテストを書く意味はありませんが、モヤッとします。

tats-u · 2023-09-29T14:22:28Z

sakura_core/charset/codechecker.h

+/*!
+ * 文字列がIVSの異体字セレクタで始まっているか判定する
+ */
+inline bool IsVariationSelector(std::wstring_view text) {


VariationSelectorは他にもBMP内のSVS（U+FE0x。CJK互換漢字や一部の絵文字などと併用）があるのでIsIVSでもいいのかもしれませんね

これちょっと迷いました。

IVS(Ideographic Variation Sequence)
漢字の異体字を表すシーケンス

IVSを構成するのは「正字1字と異体字セレクタ1字」。
異体字セレクタはVS17～VS256なので、1つの正字に対し240通りの異体字を定義できます。
VSの番号が17から始まっているのは、VS1～VS16が別用途に使われているからです。

どこで？　→　SVS(Standardized Variation Sequence)
SVSは漢字以外の異体字を定義するもので、今回の対応とは関係ありません。

単にVariationSelectorと言った場合「VS1～VS16も含まれる」と判断するのが妥当ですが、ロジックが複雑になるので「ま、いっか。」と考えることにしました。指摘されている通り、VS1～VS16はBMPに定義されるコードポイントで、この関数で判定するとやや微妙な感じになります。

IVS・SVSはそれぞれセットに対する呼び方でしたか。失礼しました。

少し長くなりますがIsSSPVariantSelector（・IsBMPVariantSelector）あたりでしょうか？

kengoide

差分を見た限りでは不可解な点はありませんでした。とにかく入れてみて様子をうかがう方針でいいと思います。

GetCharNext/GetCharPrevが「正字 + 異体字セレクタ」を1字と扱うのはおかしい気がしてきました。
これらの関数には元々同名のWin32 API関数があり、オリジナルと大きく挙動を変えるのは得策でないように思います。

この関数、どういう目的で個別に実装されたかが不思議です…。本件をコミットすれば使用箇所が1つ減りますが、他は残り一か所（対応する半角括弧の検索）しかなく、文字長1以外の結果を無視するコードなので GetSizeOfChar で事足ります。本 PR とは別件ですが、削除してコード量を減らしたいところです。

kengoide · 2023-09-30T06:48:27Z

sakura_core/parse/CWordParse.cpp

-	}
-	else if( nCharChars == 1 ){
+	ECharKind ret = CK_NULL;
+	if(const auto nCharChars = CNativeW::GetSizeOfChar(pData, pDataLen, nIdx);


重箱の隅をつつく話で恐縮なのですが、この if ブロックの表記スタイルはどのような理由で選択されたものでしょうか？
同ファイルのほかのコードとも、普段の berryzplus さんのコードとも違うスタイル（ですよね？）なので不思議に思いました。

修正する必要はありません。

カバレッジ対策で修正したコードです。

以下のようなコードで3行目が「未実行」と判定されてしまうバグがOpenCppCoverageに存在するようです。

1 | if (condition) { 2 | return 1; 3 | }

kengoide · 2023-09-30T07:06:09Z

sakura_core/parse/CWordParse.cpp

+	else if (nCharChars == 3 &&
+		IsVariationSelector(pData + nIdx + 1))
+	{
+		ret = CK_ZEN_ETC;				// 全角のその他(漢字など)


他の箇所で参照されていないので、変数を使わず直に値を返してよいと思います。

これも上記と同様にカバレッジ対策です(193行目の未実行が誤検知される。）
「修正した」とみなされるコードにだけ対処を入れてるので歪に見えるかも知れません。
代入した値は195行目のreturnで利用してます。

kengoide · 2024-02-11T03:18:40Z

特に誰からも意見がないのでマージしてみます。revert 要求があれば対応します。

berryzplus force-pushed the feature/add_ivs_support branch from 1754221 to ed1494f Compare September 23, 2023 16:52

berryzplus force-pushed the feature/add_ivs_support branch from ed1494f to 76d80fa Compare September 24, 2023 07:22

berryzplus mentioned this pull request Sep 24, 2023

結合文字（濁点・半濁点）のサポート #1938

Open

berryzplus force-pushed the feature/add_ivs_support branch 2 times, most recently from e84eeac to 3d6d3a9 Compare September 24, 2023 08:33

berryzplus marked this pull request as ready for review September 24, 2023 09:24

berryzplus marked this pull request as draft September 25, 2023 01:07

IVSの異体字セレクタに対応する

69c0676

berryzplus force-pushed the feature/add_ivs_support branch from 3d6d3a9 to 69c0676 Compare September 25, 2023 16:32

berryzplus commented Sep 28, 2023

View reviewed changes

berryzplus marked this pull request as ready for review September 28, 2023 13:57

berryzplus requested review from beru and kengoide September 29, 2023 03:46

tats-u reviewed Sep 29, 2023

View reviewed changes

kengoide approved these changes Sep 30, 2023

View reviewed changes

kengoide merged commit 7622a8e into sakura-editor:master Feb 11, 2024
22 of 25 checks passed

berryzplus deleted the feature/add_ivs_support branch March 28, 2024 09:54

berryzplus mentioned this pull request May 12, 2024

ファイル読み込みの高速化 #1951

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IVSの異体字セレクタに対応する #1937

IVSの異体字セレクタに対応する #1937

berryzplus commented Sep 23, 2023

usagisita commented Sep 23, 2023

AppVeyorBot commented Sep 23, 2023

berryzplus commented Sep 23, 2023

berryzplus commented Sep 23, 2023

AppVeyorBot commented Sep 23, 2023

AppVeyorBot commented Sep 24, 2023

AppVeyorBot commented Sep 24, 2023

AppVeyorBot commented Sep 24, 2023

tats-u commented Sep 24, 2023 •

edited

Loading

berryzplus commented Sep 24, 2023 •

edited

Loading

berryzplus commented Sep 25, 2023

tats-u commented Sep 25, 2023 •

edited

Loading

sonarcloud bot commented Sep 25, 2023

AppVeyorBot commented Sep 25, 2023

berryzplus Sep 28, 2023

tats-u Sep 29, 2023

berryzplus Sep 29, 2023

tats-u Sep 30, 2023

kengoide left a comment

kengoide Sep 30, 2023

berryzplus Oct 1, 2023

kengoide Sep 30, 2023

berryzplus Oct 1, 2023

kengoide commented Feb 11, 2024

IVSの異体字セレクタに対応する #1937

IVSの異体字セレクタに対応する #1937

Conversation

berryzplus commented Sep 23, 2023

PR対象

カテゴリ

PR の背景

仕様・動作説明

PR の影響範囲

テスト内容

関連 issue, PR

参考資料

usagisita commented Sep 23, 2023

AppVeyorBot commented Sep 23, 2023

berryzplus commented Sep 23, 2023

berryzplus commented Sep 23, 2023

AppVeyorBot commented Sep 23, 2023

AppVeyorBot commented Sep 24, 2023

AppVeyorBot commented Sep 24, 2023

AppVeyorBot commented Sep 24, 2023

tats-u commented Sep 24, 2023 • edited Loading

berryzplus commented Sep 24, 2023 • edited Loading

berryzplus commented Sep 25, 2023

tats-u commented Sep 25, 2023 • edited Loading

sonarcloud bot commented Sep 25, 2023

AppVeyorBot commented Sep 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kengoide left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kengoide commented Feb 11, 2024

tats-u commented Sep 24, 2023 •

edited

Loading

berryzplus commented Sep 24, 2023 •

edited

Loading

tats-u commented Sep 25, 2023 •

edited

Loading