タブ幅が半角単位8桁を超える場合でも下線が最後まで描画されるようにする #1673

ghost · 2021-05-21T14:21:21Z

PR の目的

タブ記号の描画処理に関する不具合を修正します。

カテゴリ

不具合修正

背景

patchunicode#1053 で指摘されている通り、タブ記号に対して下線を描画する設定が有効になっている際、
下線が半角単位8桁分しか描画されない不具合が存在します。
対策を行って、下線がタブ幅いっぱいに描画されるようにします。

再現手順

タイプ別設定を開く
スクリーンタブのレイアウト設定にて、タブ幅に「8」を超える値を設定する
カラータブの色指定にて、「TAB記号」を選択し、「下線」のチェックボックスをオンにする
タブ文字を入力する

参考画像

PR のメリット

下線がタブ幅いっぱいに描画されるようになります。

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

変更箇所は自動化された単体テストのカバー範囲外です。

仕様・動作説明

この不具合は、タブ記号の描画幅が最長で8桁に固定されているためです。
なお、タブ幅の上限値は「64」です。

以前よりタブの描画処理ではタブ幅をCLayoutMgrから取得するようになっていましたので、この PR では取得した値を半角単位桁数に換算し、タブ描画文字列の長さが換算値よりも小さければ不足分を補うようにしました。
（以下、前後比較）

タブ通常表示
- 前：タブ表示文字列を、その長さ分描画するようになっていた。
  タブ表示文字列は最大で8字分しかないため、下線はそこまでしか描画されない。
- 後：タブ表示文字列をstd::wstring型オブジェクトに入れ、タブ幅の長さが足りないときは空白を補うようにした。
タブ矢印表示
- 前：矢印の背景として描画される空白の長さが8文字固定でした。
  なお、長い矢印表示の場合、矢印だけはタブ幅いっぱいに描画されます。
- 後：タブ幅の長さ分の空白を持つstd::wstring型オブジェクトを作成し、これを描画に使うようにした。
CSVモード
- 前：タブの背景として描画される空白の長さが8文字固定でした。
- 後：タブ幅の長さ分の空白を持つstd::wstring型オブジェクトを作成し、これを描画に使うようにした。

なお、既存パッチでは不具合と関係しない他の変更が同時に実施されているように見えたため、既存パッチは利用していません。

PR の影響範囲

タブ記号の描画処理
CSVモードにおけるカンマ記号の描画処理

テスト内容

変更後のサクラエディタで前述した手順を再現し、発生しないことを確認します。
TSV/CSVモードでのテストは、添付のファイル（ 6be2aec のビルドログ）を開いてタイプ別設定を変更し、設定が反映されたときの描画結果を確認します。
各自で用意したカンマ区切り・タブ区切りテキストでも確認できます。
test-file.zip

なお、TAB通常表示およびTAB矢印表示における影響は下表の通りです。
TAB表示の設定を切り替えてそれぞれの描画を確認してください。

TAB表示 → 表示モード↓	文字指定	短い矢印	長い矢印
通常モード	下線の描画に影響	下線の描画に影響	矢印記号と下線の描画に影響
TSVモード	下線の描画に影響	下線の描画に影響	矢印記号と下線の描画に影響

確認方法

通常モード
1. サクラエディタを開く
2. タイプ別設定を変更
  スクリーンタブ：TAB幅に「8」を超える値を設定する
  スクリーンタブ：TAB表示に確認したい表示形式（上記表を参照）を設定
  カラータブ：色指定でTAB記号の下線を有効にする
3. タブ文字を入力し、描画結果を確認する
TSV モード
1. サクラエディタを開く
2. 添付した圧縮ファイルに含まれる sample_buildlog_x64_release.txt を開く
3. タイプ別設定を変更
  スクリーンタブ：折り返し方法を「折り返さない」に設定
  スクリーンタブ：TAB表示に確認したい表示形式（上記表を参照）を設定
  スクリーンタブ：レイアウトセクション一番下のコンボボックスを「TSV」に設定する
  カラータブ：色指定でTAB記号の下線を有効にする
4. 設定変更後のタブ表示を確認する
CSV モード
1. サクラエディタを開く
2. 添付した圧縮ファイルに含まれる sample_buildlog_x64_release.csv を開く
3. タイプ別設定を変更
  スクリーンタブ：折り返し方法を「折り返さない」に設定
  スクリーンタブ：レイアウトセクション一番下のコンボボックスを「CSV」に設定する
  カラータブ：色指定でTAB記号の下線を有効にする
4. 設定変更後のタブ表示を確認する

いずれのパターンにおいて、次の2点が確認できれば問題ないと思います。

下線がタブ幅いっぱいに描画されるようになった
長い矢印記号がこれまで通りタブ幅いっぱいに描画されている（CSVモードの時は矢印記号を使わないので確認不要です）

関連 issue, PR

close: patchunicode#1053
#1645 … 単位を桁数に戻した場合はタブ記号描画幅の計算処理も必ず変更しなければなりません。

ghost · 2021-05-21T14:29:33Z

sakura_core/view/figures/CFigure_Tab.cpp

-				// tabDispWidth <= 8 ? tabDispWidth : 8, // Sep. 22, 2002 genta
-				nLen,
+				szForeViewString.c_str(),
+				static_cast<UINT>(szForeViewString.length()),
 				pMetrics->GetDxArray_AllHankaku()	// FIXME:半角固定？


ここでは半角文字の文字間隔配列を取得していますが、タブ記号には全角文字も設定できるので、
GenerateDxArray2()の方が良いのかもしれません。
ただ、全角文字を指定しても現状正しく表示されているように見えるので、実害があるのか判断できませんでした。

指摘ではありませんが、タブ幅の最小値って2なんでしたっけ？

最小値に1以下を指定できるとしたら、この修正内容と関係なく問題ありな気がします。

おそらく設定できます。
タイプ別ダイアログのコードを見たところ、スピンコントロールに対して「正の整数で1～64の範囲内」という入力規則が設定されていました。

sakura/sakura_core/typeprop/CPropTypesScreen.cpp

Lines 412 to 418 in 0843eb9

if( nVal < 1 ){

nVal = 1;

}

if( nVal > 64 ){

nVal = 64;

}

::SetDlgItemInt( hwndDlg, IDC_EDIT_TABSPACE, nVal, FALSE );

タブ幅を1にしている人がいないとも限らないので、GenerateDxArray2を使用するように仕様変更（＝このPRには含めない）しましょうか？

めんどくさそうな感じっすねｗ

TAB幅に1を指定するのは趣味だと思うので、放置しましょう。

TAB幅に1を指定してあるのなら、
タブ描画に使うカスタム文字に全角文字を指定できてはいけない気がします。

TAB幅2未満のときだけ全角文字を無効とする、ないし、TAB描画文字に全角を指定したときは幅を2未満にできないようにする、みたいな修正を入れておくのがベターだと思います。
（自己責任っちゅうことで、放置でよいような・・・）

AppVeyorBot · 2021-05-21T15:26:47Z

✅ Build sakura 1.0.3782 completed (commit 6a7a98bffe by @kazasaku)

berryzplus

不具合内容は了解しました。
修正したほうがよい、も合意です。

なんとなく、必要でない箇所が直ってるように見えています。
・どのコードが原因で不具合が起きてました。だから直しました。
・修正と関連して、こういう修正を行いました。
という2点がハッキリしないのが気になってます。

berryzplus · 2021-05-22T03:31:46Z

sakura_core/view/figures/CFigure_Tab.cpp

 			::ExtTextOut(
 				gr,
 				sPos.GetDrawPos().x,
 				sPos.GetDrawPos().y,
 				ExtTextOutOption() & ~(bTrans? ETO_OPAQUE: 0),
 				&rcClip2,
-				L"        ",
-				8,
+				szBackViewString.c_str(),


不具合修正として直すべき箇所はココだけのように見えました。

sakura_core/view/figures/CFigure_Tab.cpp

ghost · 2021-05-22T04:47:38Z

sakura_core/view/figures/CFigure_Comma.cpp

 	const CTextMetrics* pMetrics=&pcView->GetTextMetrics();
 	const CTextArea* pArea=&pcView->GetTextArea();

 	int nLineHeight = pMetrics->GetHankakuDy();
-	int nCharWidth = pMetrics->GetCharPxWidth();	// Layout→Px


換算方法の見直しで使用されなくなるので除去
（CLayoutMgr由来のピクセル単位数値は、CLayoutMgrの持つ基準値で換算すべき。）

ghost · 2021-05-22T04:48:40Z

sakura_core/view/figures/CFigure_Comma.cpp

-	CLayoutXInt tabDispWidthLayout = pcView->m_pcEditDoc->m_cLayoutMgr.GetActualTsvSpace( sPos.GetDrawCol(), L',' );
-	int tabDispWidth = (Int)tabDispWidthLayout;
+	CLayoutXInt nTabLayoutWidth = pcLayoutMgr.GetActualTsvSpace(sPos.GetDrawCol(), L',');
+	size_t nTabDispWidth = static_cast<Int>(nTabLayoutWidth) / pcLayoutMgr.GetWidthPerKeta();


変数名変更
nTabLayoutWidht：タブのレイアウト幅（ピクセル単位数値）
nTabDispWidth：タブの表示幅（半角単位桁数）
nTabDispWidhtは後でsize_t型の値と演算するのでsize_tで宣言します。

ghost · 2021-05-22T04:49:47Z

sakura_core/view/figures/CFigure_Comma.cpp

-	rcClip2.right = rcClip2.left + nCharWidth * tabDispWidth;
+	rcClip2.right = rcClip2.left + static_cast<Int>(nTabLayoutWidth);


変更前はタブのピクセル幅に文字のピクセル幅を乗じていました。
（この計算結果は事実上「桁数×ピクセル単位文字幅^2」になってしまいます。）

見直したら2乗にならないことがわかりました。

sakura/sakura_core/view/CTextMetrics.h

Line 60 in 6b4f0a8

[[nodiscard]] int GetCharPxWidth() const { return 1; }

ただ、tabDispWidthの中身はピクセル単位数値なのだから、この演算をする意味はないと思います。

ghost · 2021-05-22T04:50:10Z

sakura_core/view/figures/CFigure_Comma.cpp

 	if( rcClip2.left < pArea->GetAreaLeft() ){
 		rcClip2.left = pArea->GetAreaLeft();
 	}
 	rcClip2.top = sPos.GetDrawPos().y;
 	rcClip2.bottom = sPos.GetDrawPos().y + nLineHeight;
-	int nLen = wcslen( m_pTypeData->m_szTabViewString );


Code Smell：未使用変数の定義・縮小型変換

sonarcloud · 2021-05-22T04:50:51Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

45.8% Coverage
30.3% Duplication

ghost · 2021-05-22T04:51:16Z

sakura_core/view/figures/CFigure_Comma.cpp

@@ -58,51 +56,53 @@ void CFigure_Comma::DispSpace(CGraphics& gr, DispPos* pDispPos, CEditView* pcVie
 	DispPos& sPos=*pDispPos;

 	//必要なインターフェース
+	const CLayoutMgr& pcLayoutMgr = pcView->GetDocument()->m_cLayoutMgr;


CLayoutMgrを編集可能な状態でアクセスする必要はないので、const参照経由で使用します。

ghost · 2021-05-22T04:56:36Z

sakura_core/view/figures/CFigure_Comma.cpp

-void _DispTab( CGraphics& gr, DispPos* pDispPos, const CEditView* pcView );
-
 // -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- //
-//                         CFigure_Comma                         //
+//                        CFigure_Comma                        //


CFigure_Tab.cppをコピーした名残だと思われます。
未定義かつ使用されることはないと思うので消してしまいます。

ghost · 2021-05-22T05:00:14Z

sakura_core/view/figures/CFigure_Tab.cpp

-						nCharWidth * tabDispWidth - (nPosLeft -  sPos.GetDrawPos().x),	// Tab Area一杯に 2013/4/11 Uchi
+						static_cast<Int>(nTabLayoutWidth) - (nPosLeft - sPos.GetDrawPos().x),	// Tab Area一杯に 2013/4/11 Uchi


ここもタブのピクセル幅に文字のピクセル幅を乗じていました。

ghost · 2021-05-22T05:16:35Z

加筆しました。

AppVeyorBot · 2021-05-22T05:34:15Z

✅ Build sakura 1.0.3784 completed (commit 469fcb1c26 by @kazasaku)

ghost · 2021-05-22T06:42:31Z

このPRは全く関係無いですが、ローカルでビルドしたときに次のような差分表示がVSのステータスバーに出ます。

ビルド後：変更差分「99%」
クリーン後：変更差分「16」

身に覚えのない大規模変更（99%ということは、リポジトリ内のほぼすべてのファイルということ？）なのですごい謎です。

berryzplus · 2021-05-22T07:28:31Z

身に覚えのない大規模変更（99%ということは、リポジトリ内のほぼすべてのファイルということ？）なのですごい謎です。

gitの設定が影響していませんかね？

環境	設定内容
世間一般	gitに登録された通りの改行コードで取得し、ローカルブランチの改行コード通りに登録する。
このリポジトリ	gitから取得する際にLFをCRLFに変換する。コミット時にCRLFをLFに変換して登録する。

https://qiita.com/yoshiplur/items/568d1e83c54a5c68da30
https://note.com/dafujii/n/n36c7ca892e90

ここのリポジトリはなんかおかしいので、autocrlf=trueを設定したほうが使いやすいです。「良識」に従えば、改行コードの自動変換は行うべきじゃないんですが、理解して設定するのをとやかく言うのは違うと思います。

こんな感じでコマンド叩けばそのうち治ると思います。

git config core.autoCRLF true
git config core.safeCRLF warn
git remote remove origin
git remote add origin git@github.com:sakura-editor/sakura.git
git fetch project --prune

remoteがいっぱいある場合はそれぞれ対処が必要っす。

ghost · 2021-05-22T07:48:30Z

身に覚えのない大規模変更（99%ということは、リポジトリ内のほぼすべてのファイルということ？）なのですごい謎です。

gitの設定が影響していませんかね？
環境設定内容
世間一般 gitに登録された通りの改行コードで取得し、ローカルブランチの改行コード通りに登録する。
このリポジトリ gitから取得する際にLFをCRLFに変換する。コミット時にCRLFをLFに変換して登録する。

https://qiita.com/yoshiplur/items/568d1e83c54a5c68da30
https://note.com/dafujii/n/n36c7ca892e90

ここのリポジトリはなんかおかしいので、autocrlf=trueを設定したほうが使いやすいです。「良識」に従えば、改行コードの自動変換は行うべきじゃないんですが、理解して設定するのをとやかく言うのは違うと思います。

こんな感じでコマンド叩けばそのうち治ると思います。
git config core.autoCRLF true
git config core.safeCRLF warn
git remote remove origin
git remote add origin git@github.com:sakura-editor/sakura.git
git fetch project --prune
remoteがいっぱいある場合はそれぞれ対処が必要っす。

前にcore.autocrlfをtrueにして別にcloneしたことがあったんですが、そっちはincludeGuard.ps1に差分が出て --force オプションなしではブランチを移動できなくなりました。

作業前にソリューションをクリーンして、buildディレクトリを削除すれば「0」に戻るので、当分はinputのままでいきます。

berryzplus · 2021-05-22T09:28:55Z

前にcore.autocrlfをtrueにして別にcloneしたことがあったんですが、そっちはincludeGuard.ps1に差分が出て --force オプションなしではブランチを移動できなくなりました。

作業前にソリューションをクリーンして、buildディレクトリを削除すれば「0」に戻るので、当分はinputのままでいきます。

なんかわかった気がします。 #1666 が原因ですね。

ぼくのとこでは、
repository-root/.git/info/exclude に
にDebugとかReleaseが入ってます。

berryzplus

問題なさそうに見えます。

berryzplus · 2021-05-22T13:00:59Z

#1666 じゃなくて #1670 ですね＜中間生成物の変更

ghost · 2021-05-22T13:09:44Z

確認ありがとうございます。
タブ幅計算処理はレイアウト単位の変更を実施したときに変更が必要になる（プロポーショナルフォント対応時、この2ファイルも変更されていました）ので、計算式はその時に再検討しようと思います。
静的コード解析の警告は"Complex method"と"Duplicated lines"でしたので、特に要求がなければ対応は行いません。

他に指摘等なければ近日中にマージしようと思います。

ghost · 2021-05-23T15:31:37Z

マージします。ご確認ありがとうございます。
何かありましたらお知らせください。

ghost commented May 21, 2021

View reviewed changes

beru added the 🐛bug🦋 ■バグ修正(Something isn't working) label May 22, 2021

ghost marked this pull request as ready for review May 22, 2021 03:00

berryzplus reviewed May 22, 2021

View reviewed changes

タブ幅が半角単位8桁を超える場合でも下線が最後まで描画されるようにする

6b4f0a8

ghost commented May 22, 2021

View reviewed changes

berryzplus approved these changes May 22, 2021

View reviewed changes

This was referenced May 22, 2021

UTF-8で登録されていたpowershellスクリプトを登録し直す #1675

Merged

.gitignoreにビルド構成の変更を反映する #1676

Merged

ghost merged commit 81f08b1 into sakura-editor:master May 23, 2021

ghost deleted the feature/fix_draw_length_tab_figures branch May 25, 2021 12:43

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

タブ幅が半角単位8桁を超える場合でも下線が最後まで描画されるようにする #1673

タブ幅が半角単位8桁を超える場合でも下線が最後まで描画されるようにする #1673

ghost commented May 21, 2021 •

edited by ghost

ghost May 21, 2021 •

edited by ghost

berryzplus May 22, 2021

ghost May 22, 2021 •

edited by ghost

berryzplus May 22, 2021

AppVeyorBot commented May 21, 2021

berryzplus left a comment

berryzplus May 22, 2021

ghost May 22, 2021

ghost May 22, 2021 •

edited by ghost

ghost May 22, 2021 •

edited by ghost

ghost May 22, 2021 •

edited by ghost

ghost May 22, 2021 •

edited by ghost

sonarcloud bot commented May 22, 2021 •

edited

ghost May 22, 2021

ghost May 22, 2021

ghost May 22, 2021 •

edited by ghost

ghost commented May 22, 2021

AppVeyorBot commented May 22, 2021

ghost commented May 22, 2021

berryzplus commented May 22, 2021

ghost commented May 22, 2021

berryzplus commented May 22, 2021

berryzplus left a comment

berryzplus commented May 22, 2021

ghost commented May 22, 2021

ghost commented May 23, 2021

	if( nVal < 1 ){
	nVal = 1;
	}
	if( nVal > 64 ){
	nVal = 64;
	}
	::SetDlgItemInt( hwndDlg, IDC_EDIT_TABSPACE, nVal, FALSE );

		rcClip2.right = rcClip2.left + nCharWidth * tabDispWidth;
		rcClip2.right = rcClip2.left + static_cast<Int>(nTabLayoutWidth);

		nCharWidth * tabDispWidth - (nPosLeft - sPos.GetDrawPos().x), // Tab Area一杯に 2013/4/11 Uchi
		static_cast<Int>(nTabLayoutWidth) - (nPosLeft - sPos.GetDrawPos().x), // Tab Area一杯に 2013/4/11 Uchi

タブ幅が半角単位8桁を超える場合でも下線が最後まで描画されるようにする #1673

タブ幅が半角単位8桁を超える場合でも下線が最後まで描画されるようにする #1673

Conversation

ghost commented May 21, 2021 • edited by ghost

PR の目的

カテゴリ

背景

再現手順

参考画像

PR のメリット

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

仕様・動作説明

PR の影響範囲

テスト内容

確認方法

関連 issue, PR

ghost May 21, 2021 • edited by ghost

Choose a reason for hiding this comment

berryzplus May 22, 2021

Choose a reason for hiding this comment

ghost May 22, 2021 • edited by ghost

Choose a reason for hiding this comment

berryzplus May 22, 2021

Choose a reason for hiding this comment

AppVeyorBot commented May 21, 2021

berryzplus left a comment

Choose a reason for hiding this comment

berryzplus May 22, 2021

Choose a reason for hiding this comment

ghost May 22, 2021

Choose a reason for hiding this comment

ghost May 22, 2021 • edited by ghost

Choose a reason for hiding this comment

ghost May 22, 2021 • edited by ghost

Choose a reason for hiding this comment

ghost May 22, 2021 • edited by ghost

Choose a reason for hiding this comment

ghost May 22, 2021 • edited by ghost

Choose a reason for hiding this comment

sonarcloud bot commented May 22, 2021 • edited

ghost May 22, 2021

Choose a reason for hiding this comment

ghost May 22, 2021

Choose a reason for hiding this comment

ghost May 22, 2021 • edited by ghost

Choose a reason for hiding this comment

ghost commented May 22, 2021

AppVeyorBot commented May 22, 2021

ghost commented May 22, 2021

berryzplus commented May 22, 2021

ghost commented May 22, 2021

berryzplus commented May 22, 2021

berryzplus left a comment

Choose a reason for hiding this comment

berryzplus commented May 22, 2021

ghost commented May 22, 2021

ghost commented May 23, 2021

ghost commented May 21, 2021 •

edited by ghost

ghost May 21, 2021 •

edited by ghost

ghost May 22, 2021 •

edited by ghost

ghost May 22, 2021 •

edited by ghost

ghost May 22, 2021 •

edited by ghost

ghost May 22, 2021 •

edited by ghost

ghost May 22, 2021 •

edited by ghost

sonarcloud bot commented May 22, 2021 •

edited

ghost May 22, 2021 •

edited by ghost