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

レイアウト桁位置に対するロジック桁位置の進め方を改善する #1658

Merged
4 commits merged into from May 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions sakura_core/doc/layout/CLayoutMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -873,8 +873,6 @@ void CLayoutMgr::LogicToLayout(
else{
nCharKetas = GetLayoutXOfChar( pData, nDataLen, i );
}
// if( nCharKetas == 0 ) // 削除 サロゲートペア対策 2008/7/5 Uchi
// nCharKetas = CLayoutInt(1);

//レイアウト加算
nCaretPosX += nCharKetas;
Expand Down Expand Up @@ -1007,8 +1005,6 @@ checkloop:;
else{
nCharKetas = GetLayoutXOfChar( pData, nDataLen, i );
}
// if( nCharKetas == 0 ) // 削除 サロゲートペア対策 2008/7/5 Uchi
// nCharKetas = CLayoutInt(1);

//レイアウト加算
if( nX + nCharKetas > ptLayout.GetX2() && !bEOF ){
Expand Down
43 changes: 17 additions & 26 deletions sakura_core/doc/layout/CLayoutMgr_DoLayout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ static bool _GetKeywordLength(
CLogicInt nWordLen = CLogicInt(0);
CLayoutInt nWordKetas = CLayoutInt(0);
while(nPos<cLineStr.GetLength() && IS_KEYWORD_CHAR(cLineStr.At(nPos))){
CLogicXInt nCharSize = CNativeW::GetSizeOfChar( cLineStr, nPos );
CLayoutInt k = cLayoutMgr.GetLayoutXOfChar(cLineStr, nPos);
if(0 == k)k = CLayoutInt(1);
Copy link
Member

Choose a reason for hiding this comment

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

これ、削除して大丈夫ですか? ゼロ幅スペースなどの幅0pxの文字でも1px描画するのが今の仕様であるように見えます。消してしまうと仕様変更になりませんか?

Copy link
Author

@ghost ghost May 4, 2021

Choose a reason for hiding this comment

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

現時点ではここに来るのは次の文字の時だけです( IS_KEYWORD_CHAR の仕様による)。
#$0123456789@ABCDEFGHIJKLMNOPQRSTUVWXYZ\_abcdefghijklmnopqrstuvwxyz

自分には過去に行ったサロゲートペア対応時の削除漏れに見えました。
また、この箇所以外に0桁を1桁にみなす動作をしている箇所はすべてコメントアウトされているはずです。
(このPRではCode Smellになってしまうので除去しています。)

Copy link
Member

Choose a reason for hiding this comment

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

おっしゃる通り、IS_KEYWORD_CHAR の条件があるので不要ということでよさそうですね。了解しました。


nWordLen+=1;
nWordLen += nCharSize;
nWordKetas+=k;
nPos++;
nPos += nCharSize;
}
//結果
if(nWordLen>0){
Expand Down Expand Up @@ -188,7 +188,7 @@ void CLayoutMgr::_DoKutoBurasage(SLayoutWork* pWork) const
if( _IsKinsokuPosKuto( GetMaxLineLayout() - pWork->nPosX, nCharKetas ) && IsKinsokuKuto( pWork->cLineStr.At( pWork->nPos ) ) )
{
pWork->nWordBgn = pWork->nPos;
pWork->nWordLen = 1;
pWork->nWordLen = CNativeW::GetSizeOfChar( pWork->cLineStr, pWork->nPos );
pWork->eKinsokuType = KINSOKU_TYPE_KINSOKU_KUTO;
}
}
Expand All @@ -203,24 +203,17 @@ void CLayoutMgr::_DoGyotoKinsoku(SLayoutWork* pWork, PF_OnLine pfOnLine)
&& (pWork->eKinsokuType == KINSOKU_TYPE_NONE) )
{
// 2007.09.07 kobake レイアウトとロジックの区別
CLayoutInt nCharKetas2 = GetLayoutXOfChar( pWork->cLineStr, pWork->nPos );
CLayoutInt nCharKetas3 = GetLayoutXOfChar( pWork->cLineStr, pWork->nPos+1 );
bool bLowSurrogate = false;
CLogicXInt nCharSize = CNativeW::GetSizeOfChar( pWork->cLineStr, pWork->nPos );
CLayoutXInt nCharKetas1 = GetLayoutXOfChar( pWork->cLineStr, pWork->nPos );
CLayoutXInt nCharKetas2 = GetLayoutXOfChar( pWork->cLineStr, pWork->nPos + nCharSize );

if( nCharKetas3 == 0 && pWork->nPos + 2 < pWork->cLineStr.GetLength() )
{
// サロゲートペア対策(取得した文字幅が0だったら下位側を読み取ったと判断し、次の位置に進ませる)
bLowSurrogate = true;
nCharKetas3 = GetLayoutXOfChar( pWork->cLineStr, pWork->nPos + 2 );
}

if( _IsKinsokuPosHead( GetMaxLineLayout() - pWork->nPosX, nCharKetas2, nCharKetas3 )
&& IsKinsokuHead( pWork->cLineStr.At( pWork->nPos + ( bLowSurrogate ? 2 : 1 ) ) )
if( _IsKinsokuPosHead( GetMaxLineLayout() - pWork->nPosX, nCharKetas1, nCharKetas2 )
&& IsKinsokuHead( pWork->cLineStr.At( pWork->nPos + nCharSize ) )
&& !IsKinsokuHead( pWork->cLineStr.At( pWork->nPos ) ) // 1字前が行頭禁則の対象でないこと
&& !IsKinsokuKuto( pWork->cLineStr.At( pWork->nPos ) ) ) // 1字前が句読点ぶら下げの対象でないこと
{
pWork->nWordBgn = pWork->nPos;
pWork->nWordLen = ( bLowSurrogate ? 3 : 2 );
pWork->nWordLen = nCharSize + CNativeW::GetSizeOfChar( pWork->cLineStr, pWork->nPos + nCharSize );
pWork->eKinsokuType = KINSOKU_TYPE_KINSOKU_HEAD;

(this->*pfOnLine)(pWork);
Expand All @@ -236,13 +229,14 @@ void CLayoutMgr::_DoGyomatsuKinsoku(SLayoutWork* pWork, PF_OnLine pfOnLine)
&& ( pWork->nPosX > pWork->nIndent ) // 2004.04.09 pWork->nPosXの解釈変更のため,行頭チェックも変更
&& (pWork->eKinsokuType == KINSOKU_TYPE_NONE) )
{
CLayoutInt nCharKetas2 = GetLayoutXOfChar( pWork->cLineStr, pWork->nPos );
CLayoutInt nCharKetas3 = GetLayoutXOfChar( pWork->cLineStr, pWork->nPos+1 );
CLogicXInt nCharSize = CNativeW::GetSizeOfChar( pWork->cLineStr, pWork->nPos );
CLayoutXInt nCharKetas1 = GetLayoutXOfChar( pWork->cLineStr, pWork->nPos );
CLayoutXInt nCharKetas2 = GetLayoutXOfChar( pWork->cLineStr, pWork->nPos + nCharSize );

if( _IsKinsokuPosTail( GetMaxLineLayout() - pWork->nPosX, nCharKetas2, nCharKetas3 ) && IsKinsokuTail( pWork->cLineStr.At( pWork->nPos ) ) )
if( _IsKinsokuPosTail( GetMaxLineLayout() - pWork->nPosX, nCharKetas1, nCharKetas2 ) && IsKinsokuTail( pWork->cLineStr.At( pWork->nPos ) ) )
{
pWork->nWordBgn = pWork->nPos;
pWork->nWordLen = 1;
pWork->nWordLen = nCharSize;
pWork->eKinsokuType = KINSOKU_TYPE_KINSOKU_TAIL;

(this->*pfOnLine)(pWork);
Expand All @@ -260,7 +254,7 @@ bool CLayoutMgr::_DoTab(SLayoutWork* pWork, PF_OnLine pfOnLine)
return true;
}
pWork->nPosX += nCharKetas;
pWork->nPos += CLogicInt(1);
pWork->nPos += CNativeW::GetSizeOfChar( pWork->cLineStr, pWork->nPos );
return false;
}

Expand Down Expand Up @@ -322,9 +316,6 @@ void CLayoutMgr::_MakeOneLine(SLayoutWork* pWork, PF_OnLine pfOnLine)
}
// 2007.09.07 kobake ロジック幅とレイアウト幅を区別
CLayoutInt nCharKetas = GetLayoutXOfChar( pWork->cLineStr, pWork->nPos );
// if( 0 == nCharKetas ){ // 削除 サロゲートペア対策 2008/7/5 Uchi
// nCharKetas = CLayoutInt(1);
// }

if( pWork->nPosX + nCharKetas > GetMaxLineLayout() ){
if( pWork->eKinsokuType != KINSOKU_TYPE_KINSOKU_KUTO )
Expand All @@ -336,7 +327,7 @@ void CLayoutMgr::_MakeOneLine(SLayoutWork* pWork, PF_OnLine pfOnLine)
}
}
}
pWork->nPos += 1;
pWork->nPos += CNativeW::GetSizeOfChar( pWork->cLineStr, pWork->nPos );
pWork->nPosX += nCharKetas;
}
}
Expand Down
2 changes: 0 additions & 2 deletions sakura_core/mem/CMemoryIterator.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,6 @@ class CMemoryIterator
if( m_nSpacing ){
m_nColumn_Delta += CLayoutXInt(CNativeW::GetKetaOfChar(m_pLine, m_nLineLen, m_nIndex) * m_nSpacing);
}
// if( 0 == m_nColumn_Delta ) // 削除 サロゲートペア対策 2008/7/5 Uchi
// m_nColumn_Delta = CLayoutInt(1);
}
}

Expand Down
15 changes: 8 additions & 7 deletions sakura_core/mem/CNativeW.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,19 +153,20 @@ class CNativeW final : public CNative{

public:
// -- -- staticインターフェース -- -- //
static CLogicInt GetSizeOfChar( const wchar_t* pData, int nDataLen, int nIdx ); //!< 指定した位置の文字がwchar_t何個分かを返す
static CHabaXInt GetHabaOfChar( const wchar_t* pData, int nDataLen, int nIdx,
bool bEnableExtEol, CCharWidthCache& cache = GetCharWidthCache() );
//! 指定した位置の文字がwchar_t何個分かを返す
static CLogicInt GetSizeOfChar( const wchar_t* pData, int nDataLen, int nIdx );
static CLogicInt GetSizeOfChar( const CStringRef& cStr, int nIdx )
{ return GetSizeOfChar( cStr.GetPtr(), cStr.GetLength(), nIdx ); }
Copy link
Member

Choose a reason for hiding this comment

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

追加したオーバーロードを検証するテストが欲しいです。

Copy link
Author

@ghost ghost May 4, 2021

Choose a reason for hiding this comment

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

CStringRef を扱う GetKetaOfChar / GetColmOfChar のオーバーロードにもテストが無いです。
これも追加しときます。

Copy link
Author

Choose a reason for hiding this comment

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

対応完了。

Copy link
Member

Choose a reason for hiding this comment

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

CStringRef を扱う GetKetaOfChar / GetColmOfChar のオーバーロードにもテストが無いです。

これはたぶん私のやらかしです。追加していただきありがとうございます。

//! 指定した位置の文字が半角何個分かを返す
static CKetaXInt GetKetaOfChar( const wchar_t* pData, int nDataLen, int nIdx,
CCharWidthCache& cache = GetCharWidthCache() );
static CKetaXInt GetKetaOfChar(const CStringRef& cStr, int nIdx, CCharWidthCache& cache = GetCharWidthCache())
{ return GetKetaOfChar(cStr.GetPtr(), cStr.GetLength(), nIdx, cache); }
static const wchar_t* GetCharNext( const wchar_t* pData, int nDataLen, const wchar_t* pDataCurrent ); //!< ポインタで示した文字の次にある文字の位置を返します
static const wchar_t* GetCharPrev(const wchar_t* pData, size_t nDataLen, const wchar_t* pDataCurrent); //!< ポインタで示した文字の直前にある文字の位置を返します

static CKetaXInt GetKetaOfChar( const CStringRef& cStr, int nIdx ) //!< 指定した位置の文字が半角何個分かを返す
{
return GetKetaOfChar(cStr.GetPtr(), cStr.GetLength(), nIdx);
}
static CHabaXInt GetHabaOfChar( const wchar_t* pData, int nDataLen, int nIdx,
bool bEnableExtEol, CCharWidthCache& cache = GetCharWidthCache() );
static CLayoutXInt GetColmOfChar( const wchar_t* pData,
int nDataLen, int nIdx, bool bEnableExtEol )
{ return GetHabaOfChar(pData,nDataLen,nIdx, bEnableExtEol); }
Expand Down
16 changes: 15 additions & 1 deletion tests/unittests/test-cnative.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -788,12 +788,16 @@ TEST(CNativeW, GetSizeOfChar)
{
// 基本多言語面の文字ならば1を返す。
EXPECT_EQ(CNativeW::GetSizeOfChar(L"a", 1, 0), 1);
EXPECT_EQ(CNativeW::GetSizeOfChar(CStringRef(L"a", 1), 0), 1);
// 範囲外なら0を返す。
EXPECT_EQ(CNativeW::GetSizeOfChar(L"", 0, 0), 0);
EXPECT_EQ(CNativeW::GetSizeOfChar(CStringRef(L"", 0), 0), 0);
// 上位・下位サロゲートの組み合わせであれば2を返す。
EXPECT_EQ(CNativeW::GetSizeOfChar(L"\xd83c\xdf38", 2, 0), 2);
EXPECT_EQ(CNativeW::GetSizeOfChar(CStringRef(L"\xd83c\xdf38", 2), 0), 2);
// 指定位置が下位サロゲートならその他の文字と同様に1を返す。
EXPECT_EQ(CNativeW::GetSizeOfChar(L"\xd83c\xdf38", 2, 1), 1);
EXPECT_EQ(CNativeW::GetSizeOfChar(CStringRef(L"\xd83c\xdf38", 2), 1), 1);
}

/*!
Expand All @@ -804,12 +808,16 @@ TEST(CNativeW, GetKetaOfChar)
{
// 範囲外なら0を返す。
EXPECT_EQ(CNativeW::GetKetaOfChar(L"", 0, 0), 0);
EXPECT_EQ(CNativeW::GetKetaOfChar(CStringRef(L"", 0), 0), 0);
// 上位サロゲートなら2を返す。
EXPECT_EQ(CNativeW::GetKetaOfChar(L"\xd83c\xdf38", 2, 0), 2);
EXPECT_EQ(CNativeW::GetKetaOfChar(CStringRef(L"\xd83c\xdf38", 2), 0), 2);
// 上位サロゲートに続く下位サロゲートであれば0を返す。
EXPECT_EQ(CNativeW::GetKetaOfChar(L"\xd83c\xdf38", 2, 1), 0);
EXPECT_EQ(CNativeW::GetKetaOfChar(CStringRef(L"\xd83c\xdf38", 2), 1), 0);
// 下位サロゲートだけなら2を返す。
EXPECT_EQ(CNativeW::GetKetaOfChar(L"\xdf38", 1, 0), 2);
EXPECT_EQ(CNativeW::GetKetaOfChar(CStringRef(L"\xdf38", 1), 0), 2);

// サクラエディタでは Unicode で表現できない文字コードの破壊を防ぐため、
// 不明バイトを下位サロゲートにマップして保持している。
Expand All @@ -819,19 +827,25 @@ TEST(CNativeW, GetKetaOfChar)
// https://sourceforge.net/p/sakura-editor/patchunicode/57/
// http://sakura-editor.sourceforge.net/cgi-bin/cyclamen/cyclamen.cgi?log=unicode&v=833
EXPECT_EQ(CNativeW::GetKetaOfChar(L"\xdbff", 1, 0), 2);
for (wchar_t ch = 0xdc00; ch <= 0xdcff; ++ch)
EXPECT_EQ(CNativeW::GetKetaOfChar(CStringRef(L"\xdbff", 1), 0), 2);
for (wchar_t ch = 0xdc00; ch <= 0xdcff; ++ch) {
EXPECT_EQ(CNativeW::GetKetaOfChar(&ch, 1, 0), 1);
EXPECT_EQ(CNativeW::GetKetaOfChar(CStringRef(&ch, 1), 0), 1);
}
EXPECT_EQ(CNativeW::GetKetaOfChar(L"\xdd00", 1, 0), 2);
EXPECT_EQ(CNativeW::GetKetaOfChar(CStringRef(L"\xdd00", 1), 0), 2);

// 文字が半角なら1を返す。
EXPECT_EQ(CNativeW::GetKetaOfChar(L"a", 1, 0), 1);
EXPECT_EQ(CNativeW::GetKetaOfChar(CStringRef(L"a", 1), 0), 1);

// 文字が全角なら2を返す。
class FakeCache : public CCharWidthCache {
public:
bool CalcHankakuByFont(wchar_t c) override { return false; }
} cache;
EXPECT_EQ(CNativeW::GetKetaOfChar(L"あ", 1, 0, cache), 2);
EXPECT_EQ(CNativeW::GetKetaOfChar(CStringRef(L"あ", 1), 0, cache), 2);
}

/*!
Expand Down