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.

Sign up for GitHub

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

freetype: fix for huge CtoL param. #3585

Open
wants to merge 2 commits into
base: 4.x
Choose a base branch
from
Open

freetype: fix for huge CtoL param. #3585

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d435884
freetype: fix for huge CtoL param.
Kumataro Oct 28, 2023
2fab0be
fix for review, and re-considering the valid range of iCtoL.
Kumataro Nov 9, 2023
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
61 changes: 55 additions & 6 deletions modules/freetype/src/freetype.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -723,10 +723,28 @@ int FreeType2Impl::coFn( const FT_Vector *cnt,

PathUserData *p = (PathUserData *)user;

// Calicurate upper limit of CtoL.
// It is same as total length of polylines{ mOldP, cnt, to }.

const double dx1 = p->mOldP.x - cnt->x;
const double dy1 = p->mOldP.y - cnt->y;
const double dx2 = cnt->x - to->x;
const double dy2 = cnt->y - to->y;

const int maxL = static_cast<int>(
std::pow( dx1 * dx1 + dy1 * dy1, 0.5 ) +
std::pow( dx2 * dx2 + dy2 * dy2, 0.5 )
Copy link
Contributor

Choose a reason for hiding this comment

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

pow 0.5 is sqrt. I propose to use sqrt as it's faster and more obvious.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your review! Yes, it is better, I fixed it.

);

const int iCtoL = std::min( maxL , p->mCtoL );

// Avoid to push_back() same as previous points.
Point prevPt(INT_MIN, INT_MIN);

// Bezier to Line
for(int i = 0;i <= p->mCtoL; i++){
for(int i = 0;i <= iCtoL; i++){
// Split Bezier to lines ( in FreeType coordinates ).
double u = (double)i * 1.0 / (p->mCtoL) ;
double u = (double)i * 1.0 / (iCtoL) ;
Copy link
Contributor

Choose a reason for hiding this comment

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

*1.0 is redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I fixed it. And I replaced from (double)i that is C-style-cast, to static_cast<double>(i).

double nu = 1.0 - u;
double p0 = nu * nu;
double p1 = 2.0 * u * nu;
Expand All @@ -736,7 +754,12 @@ int FreeType2Impl::coFn( const FT_Vector *cnt,
double Y = (p->mOldP.y) * p0 + cnt->y * p1 + to->y * p2;

// Store points to draw( in OpenCV coordinates ).
p->mPts.push_back( Point ( ftd(X), ftd(Y) ) );
Point pt ( ftd(X), ftd(Y) );
if ( pt != prevPt )
{
p->mPts.push_back( pt );
prevPt = pt;
}
}
p->mOldP = *to;
return 0;
Expand All @@ -754,10 +777,31 @@ int FreeType2Impl::cuFn( const FT_Vector *cnt1,

PathUserData *p = (PathUserData *)user;

// Calicurate upper limit of CtoL.
// It is same as total length of polylines{ mOldP, cnt1, cnt2, to }.

const double dx1 = p->mOldP.x - cnt1->x;
const double dy1 = p->mOldP.y - cnt1->y;
const double dx2 = cnt1->x - cnt2->x;
const double dy2 = cnt1->y - cnt2->y;
const double dx3 = cnt2->x - to->x;
const double dy3 = cnt2->y - to->y;

const int maxL = static_cast<int>(
std::pow( dx1 * dx1 + dy1 * dy1, 0.5 ) +
std::pow( dx2 * dx2 + dy2 * dy2, 0.5 ) +
std::pow( dx3 * dx3 + dy3 * dy3, 0.5 )
Copy link
Contributor

Choose a reason for hiding this comment

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

the same for pow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed it, thank you!

);

const int iCtoL = std::min( maxL, p->mCtoL );

// Avoid to push_back() same as previous points.
Point prevPt(INT_MIN, INT_MIN);

// Bezier to Line
for(int i = 0; i <= p->mCtoL ;i++){
for(int i = 0; i <= iCtoL ;i++){
// Split Bezier to lines ( in FreeType coordinates ).
double u = (double)i * 1.0 / (p->mCtoL) ;
double u = (double)i * 1.0 / (iCtoL) ;
Copy link
Contributor

Choose a reason for hiding this comment

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

*1.0 is redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed it, thank you!

double nu = 1.0 - u;
double p0 = nu * nu * nu;
double p1 = 3.0 * u * nu * nu;
Expand All @@ -770,7 +814,12 @@ int FreeType2Impl::cuFn( const FT_Vector *cnt1,
(cnt2->y ) * p2 + (to->y ) * p3;

// Store points to draw( in OpenCV coordinates ).
p->mPts.push_back( Point ( ftd(X), ftd(Y) ) );
Point pt ( ftd(X), ftd(Y) );
if ( pt != prevPt )
{
p->mPts.push_back( pt );
prevPt = pt;
}
}
p->mOldP = *to;
return 0;
Expand Down
99 changes: 92 additions & 7 deletions modules/freetype/test/test_basic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,59 @@ struct MattypeParams
bool expect_success;
};

::std::ostream& operator<<(::std::ostream& os, const MattypeParams& prm) {
return os << prm.title;
::std::ostream& operator<<(::std::ostream& os, const MattypeParams& prm)
{
os << "title = " << prm.title << " ";
os << "mattype = ";

switch( CV_MAT_DEPTH( prm.mattype ) )
{
case CV_8U: os << "CV_8U" ; break;
case CV_8S: os << "CV_8S" ; break;
case CV_16U: os << "CV_16U"; break;
case CV_16S: os << "CV_16S"; break;
case CV_32S: os << "CV_32S"; break;
case CV_32F: os << "CV_32F"; break;
case CV_64F: os << "CV_64F"; break;
case CV_16F: os << "CV_16F"; break;

// OpenCV5
#ifdef CV_16BF
case CV_16BF: os << "CV_16BF"; break;
#endif
#ifdef CV_Bool
case CV_Bool: os << "CV_Bool"; break;
#endif
#ifdef CV_64U
case CV_64U: os << "CV_64U"; break;
#endif
#ifdef CV_64S
case CV_64S: os << "CV_64S"; break;
#endif
#ifdef CV_32U
case CV_32U: os << "CV_32U"; break;
#endif
default: os << "CV UNKNOWN_DEPTH(" << CV_MAT_DEPTH( prm.mattype ) << ")" ; break;
}

switch( CV_MAT_CN( prm.mattype ) )
{
case 1: os << "C1" ; break;
case 2: os << "C2" ; break;
case 3: os << "C3" ; break;
case 4: os << "C4" ; break;
default: os << "UNKNOWN_CN" << CV_MAT_CN( prm.mattype ) << ")" ; break;
}

os << " expected = " << (prm.expect_success? "true": "false") ;

return os;
}

const MattypeParams mattype_list[] =
{
{ "CV_8UC1", CV_8UC1, true}, { "CV_8UC2", CV_8UC2, false},
{ "CV_8UC3", CV_8UC3, true}, { "CV_8UC4", CV_8UC4, true},

{ "CV_8SC1", CV_8SC1, false}, { "CV_8SC2", CV_8SC2, false},
{ "CV_8SC3", CV_8SC3, false}, { "CV_8SC4", CV_8SC4, false},
{ "CV_16UC1", CV_16UC1, false}, { "CV_16UC2", CV_16UC2, false},
Expand All @@ -34,7 +78,35 @@ const MattypeParams mattype_list[] =
{ "CV_64FC1", CV_64FC1, false}, { "CV_64FC2", CV_64FC2, false},
{ "CV_64FC3", CV_64FC3, false}, { "CV_64FC4", CV_64FC4, false},
{ "CV_16FC1", CV_16FC1, false}, { "CV_16FC2", CV_16FC2, false},
{ "CV_16FC3", CV_16FC3, false}, { "CV_16FC4", CV_16FC4, false},
{ "CV_16FC3", CV_16FC3, false}, { "CV_16FC4", CV_16FC4, false}

// OpenCV5
#ifdef CV_16BF
,
{ "CV_16BFC1", CV_16BFC1, false}, { "CV_16FC2", CV_16BFC2, false},
{ "CV_16BFC3", CV_16BFC3, false}, { "CV_16FC4", CV_16BFC4, false}
#endif
#ifdef CV_Bool
,
{ "CV_BoolC1", CV_BoolC1, false}, { "CV_BoolC2", CV_BoolC2, false},
{ "CV_BoolC3", CV_BoolC3, false}, { "CV_BoolC4", CV_BoolC4, false}
#endif
#ifdef CV_64U
,
{ "CV_64UC1", CV_64UC1, false}, { "CV_64UC2", CV_64UC2, false},
{ "CV_64UC3", CV_64UC3, false}, { "CV_64UC4", CV_64UC4, false}
#endif
#ifdef CV_64S
,
{ "CV_64SC1", CV_64SC1, false}, { "CV_64SC2", CV_64SC2, false},
{ "CV_64SC3", CV_64SC3, false}, { "CV_64SC4", CV_64SC4, false}
#endif
#ifdef CV_32U
,
{ "CV_32UC1", CV_32UC1, false}, { "CV_32UC2", CV_32UC2, false},
{ "CV_32UC3", CV_32UC3, false}, { "CV_32UC4", CV_32UC4, false}
#endif

};

/******************
Expand Down Expand Up @@ -152,19 +224,32 @@ TEST_P(ctol_range, success)
EXPECT_NO_THROW( ft2->putText(dst, "CtoL", Point( 0, 50), 50, col, 1, LINE_4, true ) );
EXPECT_NO_THROW( ft2->putText(dst, "LINE_4: oOpPqQ", Point( 40, 100), 50, col, 1, LINE_4, true ) );
EXPECT_NO_THROW( ft2->putText(dst, "LINE_8: oOpPqQ", Point( 40, 150), 50, col, 1, LINE_8, true ) );
EXPECT_NO_THROW( ft2->putText(dst, "LINE_AA:oOpPqQ", Point( 40, 150), 50, col, 1, LINE_AA, true ) );
EXPECT_NO_THROW( ft2->putText(dst, "LINE_AA:oOpPqQ", Point( 40, 200), 50, col, 1, LINE_AA, true ) );

if (cvtest::debugLevel > 0 )
{
imwrite( cv::format("CtoL%d-MatType.png", GetParam()) , dst );
}
}

const int ctol_list[] =
{
1,
2,
4,
8,
16,
32,
64,
128,
// INT_MAX -1, // Hang-up
// INT_MAX // Hang-up
256,
512,
1024,
2048,
4096,
8192,
INT_MAX -1,
INT_MAX
};

INSTANTIATE_TEST_CASE_P(Freetype_setSplitNumber, ctol_range,
Expand Down