diff --git a/docs/changes.txt b/docs/changes.txt index 156e0bf467..e590145773 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -68,6 +68,9 @@ Changes in behaviour not resulting in compilation errors - wxGTK wxTextCtrl doesn't generate any wxEVT_TEXT when it's created with non-empty value, for consistency with the other ports. +- wxDC::GetTextExtent() returns height of 0 for empty string in wxGTK and wxOSX + too now, for consistency with wxMSW and other kinds of wxDC. + - wxMSW wxToolBar height now adapts to the height of embedded controls, making the toolbar taller if necessary, rather than making the controls smaller. To return to the previous behaviour, you need to explicitly create controls of diff --git a/include/wx/private/textmeasure.h b/include/wx/private/textmeasure.h index db96dcd38f..3671a87180 100644 --- a/include/wx/private/textmeasure.h +++ b/include/wx/private/textmeasure.h @@ -128,6 +128,10 @@ protected: wxCoord *descent = NULL, wxCoord *externalLeading = NULL); + // Get line height: used when the line is empty because CallGetTextExtent() + // would just return (0, 0) in this case. + int GetEmptyLineHeight(); + // Return a valid font: if one was given to us in the ctor, use this one, // otherwise use the current font of the associated wxDC or wxWindow. wxFont GetFont() const; diff --git a/interface/wx/dc.h b/interface/wx/dc.h index 9a2cbe21da..527d97eb64 100644 --- a/interface/wx/dc.h +++ b/interface/wx/dc.h @@ -892,6 +892,12 @@ public: used for the text extent calculation, otherwise the currently selected font is used. + If @a string is empty, its horizontal extent is 0 but, for convenience + when using this function for allocating enough space for a possibly + multi-line string, its vertical extent is the same as the height of an + empty line of text. Please note that this behaviour differs from that + of GetTextExtent(). + @note This function works with both single-line and multi-line strings. @beginWxPerlOnly @@ -954,6 +960,8 @@ public: used for the text extent calculation. Otherwise the currently selected font is. + If @a string is empty, its extent is 0 in both directions, as expected. + @note This function only works with single-line strings. @beginWxPerlOnly diff --git a/src/common/dcgraph.cpp b/src/common/dcgraph.cpp index fa5a7558be..5da2fe531d 100644 --- a/src/common/dcgraph.cpp +++ b/src/common/dcgraph.cpp @@ -1234,9 +1234,21 @@ void wxGCDCImpl::DoGetTextExtent( const wxString &str, wxCoord *width, wxCoord * m_graphicContext->SetFont( *theFont, m_textForegroundColour ); } - wxDouble h , d , e , w; + wxDouble w wxDUMMY_INITIALIZE(0), + h wxDUMMY_INITIALIZE(0), + d wxDUMMY_INITIALIZE(0), + e wxDUMMY_INITIALIZE(0); - m_graphicContext->GetTextExtent( str, &w, &h, &d, &e ); + // Don't pass non-NULL pointers for the parts we don't need, this could + // result in doing extra unnecessary work inside GetTextExtent(). + m_graphicContext->GetTextExtent + ( + str, + width ? &w : NULL, + height ? &h : NULL, + descent ? &d : NULL, + externalLeading ? &e : NULL + ); if ( height ) *height = (wxCoord)(h+0.5); diff --git a/src/common/textmeasurecmn.cpp b/src/common/textmeasurecmn.cpp index 16448d2664..8ad8e0a312 100644 --- a/src/common/textmeasurecmn.cpp +++ b/src/common/textmeasurecmn.cpp @@ -100,25 +100,48 @@ void wxTextMeasureBase::GetTextExtent(const wxString& string, CallGetTextExtent(string, width, height, descent, externalLeading); } +int wxTextMeasureBase::GetEmptyLineHeight() +{ + int dummy, height; + CallGetTextExtent(wxS("W"), &dummy, &height); + return height; +} + void wxTextMeasureBase::GetMultiLineTextExtent(const wxString& text, wxCoord *width, wxCoord *height, wxCoord *heightOneLine) { + // To make the code simpler, make sure that the width and height pointers + // are always valid, by making them point to dummy variables if necessary. + int unusedWidth, unusedHeight; + if ( !width ) + width = &unusedWidth; + if ( !height ) + height = &unusedHeight; + + *width = 0; + *height = 0; + + MeasuringGuard guard(*this); + // It's noticeably faster to handle the case of a string which isn't // actually multiline specially here, to skip iteration above in this case. if ( text.find('\n') == wxString::npos ) { - GetTextExtent(text, width, height); - if ( heightOneLine && height ) + // This case needs to be handled specially as we're supposed to return + // a non-zero height even for empty string. + if ( text.empty() ) + *height = GetEmptyLineHeight(); + else + CallGetTextExtent(text, width, height); + + if ( heightOneLine ) *heightOneLine = *height; return; } - MeasuringGuard guard(*this); - - wxCoord widthTextMax = 0, widthLine, - heightTextTotal = 0, heightLineDefault = 0, heightLine = 0; + wxCoord widthLine, heightLine = 0, heightLineDefault = 0; wxString::const_iterator lineStart = text.begin(); for ( wxString::const_iterator pc = text.begin(); ; ++pc ) @@ -136,21 +159,18 @@ void wxTextMeasureBase::GetMultiLineTextExtent(const wxString& text, if ( !heightLineDefault ) heightLineDefault = heightLine; + // and if we hadn't had any previous one neither, compute it now if ( !heightLineDefault ) - { - // but we don't know it yet - choose something reasonable - int dummy; - CallGetTextExtent(wxS("W"), &dummy, &heightLineDefault); - } + heightLineDefault = GetEmptyLineHeight(); - heightTextTotal += heightLineDefault; + *height += heightLineDefault; } else { CallGetTextExtent(wxString(lineStart, pc), &widthLine, &heightLine); - if ( widthLine > widthTextMax ) - widthTextMax = widthLine; - heightTextTotal += heightLine; + if ( widthLine > *width ) + *width = widthLine; + *height += heightLine; } if ( pc == text.end() ) @@ -165,10 +185,6 @@ void wxTextMeasureBase::GetMultiLineTextExtent(const wxString& text, } } - if ( width ) - *width = widthTextMax; - if ( height ) - *height = heightTextTotal; if ( heightOneLine ) *heightOneLine = heightLine; } diff --git a/src/generic/graphicc.cpp b/src/generic/graphicc.cpp index b98a62ecf7..0eb6646099 100644 --- a/src/generic/graphicc.cpp +++ b/src/generic/graphicc.cpp @@ -2866,7 +2866,7 @@ void wxCairoContext::GetTextExtent( const wxString &str, wxDouble *width, wxDoub fe.height = fe.ascent + fe.descent; } - if (height) + if (height && !str.empty()) *height = fe.height; if ( descent ) *descent = fe.descent; diff --git a/tests/asserthelper.cpp b/tests/asserthelper.cpp index 7b25cec2d6..73a980cac1 100644 --- a/tests/asserthelper.cpp +++ b/tests/asserthelper.cpp @@ -49,7 +49,7 @@ std::ostream& operator<<(std::ostream& os, const wxColour& c) std::ostream& operator<<(std::ostream& os, const wxSize& s) { - os << s.x << "x" << s.y; + os << s.x << "*" << s.y; return os; } diff --git a/tests/graphics/measuring.cpp b/tests/graphics/measuring.cpp index 397f586421..bb185247af 100644 --- a/tests/graphics/measuring.cpp +++ b/tests/graphics/measuring.cpp @@ -33,44 +33,7 @@ #include "wx/dcps.h" #include "wx/metafile.h" -// ---------------------------------------------------------------------------- -// test class -// ---------------------------------------------------------------------------- - -class MeasuringTextTestCase : public CppUnit::TestCase -{ -public: - MeasuringTextTestCase() { } - -private: - CPPUNIT_TEST_SUITE( MeasuringTextTestCase ); - CPPUNIT_TEST( DCGetTextExtent ); - CPPUNIT_TEST( LeadingAndDescent ); - CPPUNIT_TEST( WindowGetTextExtent ); - CPPUNIT_TEST( GetPartialTextExtent ); -#ifdef TEST_GC - CPPUNIT_TEST( GraphicsGetTextExtent ); -#endif // TEST_GC - CPPUNIT_TEST_SUITE_END(); - - void DCGetTextExtent(); - void LeadingAndDescent(); - void WindowGetTextExtent(); - - void GetPartialTextExtent(); - -#ifdef TEST_GC - void GraphicsGetTextExtent(); -#endif // TEST_GC - - wxDECLARE_NO_COPY_CLASS(MeasuringTextTestCase); -}; - -// register in the unnamed registry so that these tests are run by default -CPPUNIT_TEST_SUITE_REGISTRATION( MeasuringTextTestCase ); - -// also include in its own registry so that these tests can be run alone -CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( MeasuringTextTestCase, "MeasuringTextTestCase" ); +#include "asserthelper.h" // ---------------------------------------------------------------------------- // helper for XXXTextExtent() methods @@ -86,11 +49,14 @@ struct GetTextExtentTester int y; obj.GetTextExtent("H", NULL, &y); - CPPUNIT_ASSERT( y > 1 ); + CHECK( y > 1 ); wxSize size = obj.GetTextExtent("Hello"); - CPPUNIT_ASSERT( size.x > 1 ); - CPPUNIT_ASSERT_EQUAL( y, size.y ); + CHECK( size.x > 1 ); + CHECK( size.y == y ); + + // Test that getting text extent of an empty string returns (0, 0). + CHECK( obj.GetTextExtent(wxString()) == wxSize() ); } }; @@ -98,7 +64,7 @@ struct GetTextExtentTester // tests themselves // ---------------------------------------------------------------------------- -void MeasuringTextTestCase::DCGetTextExtent() +TEST_CASE("wxDC::GetTextExtent", "[dc][text-extent]") { wxClientDC dc(wxTheApp->GetTopWindow()); @@ -107,9 +73,15 @@ void MeasuringTextTestCase::DCGetTextExtent() int w; dc.GetMultiLineTextExtent("Good\nbye", &w, NULL); const wxSize sz = dc.GetTextExtent("Good"); - CPPUNIT_ASSERT_EQUAL( sz.x, w ); + CHECK( w == sz.x ); - CPPUNIT_ASSERT( dc.GetMultiLineTextExtent("Good\nbye").y >= 2*sz.y ); + CHECK( dc.GetMultiLineTextExtent("Good\nbye").y >= 2*sz.y ); + + // Check that empty lines get counted + CHECK( dc.GetMultiLineTextExtent("\n\n\n").y >= 3*sz.y ); + + // And even empty strings count like one line. + CHECK( dc.GetMultiLineTextExtent(wxString()) == wxSize(0, sz.y) ); // Test the functions with some other DC kinds also. #if wxUSE_PRINTING_ARCHITECTURE && wxUSE_POSTSCRIPT @@ -128,19 +100,19 @@ void MeasuringTextTestCase::DCGetTextExtent() #endif } -void MeasuringTextTestCase::LeadingAndDescent() +TEST_CASE("wxDC::LeadingAndDescent", "[dc][text-extent]") { wxClientDC dc(wxTheApp->GetTopWindow()); // Retrieving just the descent should work. int descent = -17; dc.GetTextExtent("foo", NULL, NULL, &descent); - CPPUNIT_ASSERT( descent != -17 ); + CHECK( descent != -17 ); // Same for external leading. int leading = -289; dc.GetTextExtent("foo", NULL, NULL, NULL, &leading); - CPPUNIT_ASSERT( leading != -289 ); + CHECK( leading != -289 ); // And both should also work for the empty string as they retrieve the // values valid for the entire font and not just this string. @@ -148,46 +120,46 @@ void MeasuringTextTestCase::LeadingAndDescent() leading2; dc.GetTextExtent("", NULL, NULL, &descent2, &leading2); - CPPUNIT_ASSERT_EQUAL( descent, descent2 ); - CPPUNIT_ASSERT_EQUAL( leading, leading2 ); + CHECK( descent2 == descent ); + CHECK( leading2 == leading ); } -void MeasuringTextTestCase::WindowGetTextExtent() +TEST_CASE("wxWindow::GetTextExtent", "[window][text-extent]") { wxWindow* const win = wxTheApp->GetTopWindow(); GetTextExtentTester testWin(*win); } -void MeasuringTextTestCase::GetPartialTextExtent() +TEST_CASE("wxDC::GetPartialTextExtent", "[dc][text-extent][partial]") { wxClientDC dc(wxTheApp->GetTopWindow()); wxArrayInt widths; - CPPUNIT_ASSERT( dc.GetPartialTextExtents("Hello", widths) ); - CPPUNIT_ASSERT_EQUAL( 5, widths.size() ); - CPPUNIT_ASSERT_EQUAL( widths[0], dc.GetTextExtent("H").x ); - CPPUNIT_ASSERT_EQUAL( widths[4], dc.GetTextExtent("Hello").x ); + REQUIRE( dc.GetPartialTextExtents("Hello", widths) ); + REQUIRE( widths.size() == 5 ); + CHECK( widths[0] == dc.GetTextExtent("H").x ); + CHECK( widths[4] == dc.GetTextExtent("Hello").x ); } #ifdef TEST_GC -void MeasuringTextTestCase::GraphicsGetTextExtent() +TEST_CASE("wxGC::GetTextExtent", "[dc][text-extent]") { wxGraphicsRenderer* renderer = wxGraphicsRenderer::GetDefaultRenderer(); - CPPUNIT_ASSERT(renderer); + REQUIRE(renderer); wxGraphicsContext* context = renderer->CreateMeasuringContext(); - CPPUNIT_ASSERT(context); + REQUIRE(context); wxFont font(12, wxFONTFAMILY_DEFAULT, wxFONTSTYLE_NORMAL, wxFONTWEIGHT_NORMAL); - CPPUNIT_ASSERT(font.IsOk()); + REQUIRE(font.IsOk()); context->SetFont(font, *wxBLACK); double width, height, descent, externalLeading = 0.0; context->GetTextExtent("x", &width, &height, &descent, &externalLeading); delete context; // TODO: Determine a way to make these tests more robust. - CPPUNIT_ASSERT(width > 0.0); - CPPUNIT_ASSERT(height > 0.0); + CHECK(width > 0.0); + CHECK(height > 0.0); }