From a37da0fa555e3399a36e0c7cbc4ea96a4bf4d4f0 Mon Sep 17 00:00:00 2001 From: Francesco Montorsi Date: Wed, 10 Mar 2010 13:57:47 +0000 Subject: [PATCH] Fix function wxControlBase::DoEllipsizeSingleLine to really make sure that the ellipsized string takes less pixels than maxFinalWidthPx. Add comments to explain in more details what the function does and in particular the valid ranges of all internal variables; fix in that regard both the code of both wxELLIPSIZE_START, wxELLIPSIZE_MIDDLE and wxELLIPSIZE_END. Add more asserts to check the valid ranges and turn a couple of time-expensive checks in level-2 asserts. Add a test unit for the wxControl::Ellipsize function. Fix minor details in the docs of wxControl::Ellipsize. Closes #11567. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@63660 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- interface/wx/control.h | 12 ++- src/common/ctrlcmn.cpp | 137 ++++++++++++++++++------------- tests/Makefile.in | 4 + tests/graphics/ellipsization.cpp | 78 ++++++++++++++++++ tests/makefile.bcc | 4 + tests/makefile.gcc | 4 + tests/makefile.vc | 4 + tests/makefile.wat | 4 + tests/test.bkl | 1 + tests/test_test_gui.dsp | 4 + tests/test_vc7_test_gui.vcproj | 3 + tests/test_vc8_test_gui.vcproj | 4 + tests/test_vc9_test_gui.vcproj | 4 + 13 files changed, 203 insertions(+), 60 deletions(-) create mode 100644 tests/graphics/ellipsization.cpp diff --git a/interface/wx/control.h b/interface/wx/control.h index d96a9bc28c..5823f31262 100644 --- a/interface/wx/control.h +++ b/interface/wx/control.h @@ -96,6 +96,10 @@ public: /** Replaces parts of the @a label string with ellipsis, if needed, so that it doesn't exceed @a maxWidth. + + Note that this functions is guaranteed to always returns a string + whose rendering on the given DC takes less than @a maxWidth pixels + in horizontal. @param label The string to ellipsize @@ -103,11 +107,15 @@ public: The DC used to retrieve the character widths through the wxDC::GetPartialTextExtents() function. @param mode - The ellipsization modes. See ::wxEllipsizeMode. + The ellipsization mode. This is the setting which determines + which part of the string should be replaced by the ellipsis. + See ::wxEllipsizeMode enumeration values for more info. @param maxWidth The maximum width of the returned string in pixels. + This argument determines how much characters of the string need to + be removed (and replaced by ellipsis). @param flags - One or more of the ::wxEllipsize + One or more of the ::wxEllipsizeFlags enumeration values combined. */ static wxString Ellipsize(const wxString& label, const wxDC& dc, wxEllipsizeMode mode, int maxWidth, diff --git a/src/common/ctrlcmn.cpp b/src/common/ctrlcmn.cpp index a96c7a89b4..851237fe56 100644 --- a/src/common/ctrlcmn.cpp +++ b/src/common/ctrlcmn.cpp @@ -242,8 +242,8 @@ wxString wxControlBase::DoEllipsizeSingleLine(const wxString& curLine, const wxD { wxASSERT_MSG(replacementWidthPx > 0 && marginWidthPx > 0, "Invalid parameters"); - wxASSERT_MSG(!curLine.Contains('\n'), - "Use Ellipsize() instead!"); + wxASSERT_LEVEL_2_MSG(!curLine.Contains('\n'), + "Use Ellipsize() instead!"); wxASSERT_MSG( mode != wxELLIPSIZE_NONE, "shouldn't be called at all then" ); @@ -261,92 +261,107 @@ wxString wxControlBase::DoEllipsizeSingleLine(const wxString& curLine, const wxD wxASSERT(charOffsetsPx.GetCount() == len); + // NOTE: charOffsetsPx[n] is the width in pixels of the first n characters (with the last one INCLUDED) + // thus charOffsetsPx[len-1] is the total width of the string size_t totalWidthPx = charOffsetsPx.Last(); if ( totalWidthPx <= (size_t)maxFinalWidthPx ) return curLine; // we don't need to do any ellipsization! - int excessPx = totalWidthPx - maxFinalWidthPx + - replacementWidthPx + - marginWidthPx; // security margin (NEEDED!) - wxASSERT(excessPx>0); + int excessPx = wxMin(totalWidthPx - maxFinalWidthPx + + replacementWidthPx + + marginWidthPx, // security margin + totalWidthPx); + wxASSERT(excessPx>0); // excessPx should be in the [1;totalWidthPx] range - // remove characters in excess - size_t initialCharToRemove, // index of first char to erase - nCharsToRemove; // how many chars do we need to erase? + // REMEMBER: indexes inside the string have a valid range of [0;len-1] if not otherwise constrained + // lengths/counts of characters (e.g. nCharsToRemove) have a valid range of [0;len] if not otherwise constrained + // NOTE: since this point we know we have for sure a non-empty string from which we need + // to remove _at least_ one character (thus nCharsToRemove below is constrained to be >= 1) + size_t initialCharToRemove, // index of first character to erase, valid range is [0;len-1] + nCharsToRemove; // how many chars do we need to erase? valid range is [1;len-initialCharToRemove] + + // let's compute the range of characters to remove depending on the ellipsization mode: switch (mode) { case wxELLIPSIZE_START: initialCharToRemove = 0; - for ( nCharsToRemove=0; - nCharsToRemove < len && charOffsetsPx[nCharsToRemove] < excessPx; + for ( nCharsToRemove = 1; + nCharsToRemove < len && charOffsetsPx[nCharsToRemove-1] < excessPx; nCharsToRemove++ ) ; break; case wxELLIPSIZE_MIDDLE: { - // the start & end of the removed span of chars + // NOTE: the following piece of code works also when len == 1 + + // start the removal process from the middle of the string + // i.e. separe the string in three parts: + // - the first one to preserve, valid range [0;initialCharToRemove-1] or the empty range if initialCharToRemove==0 + // - the second one to remove, valid range [initialCharToRemove;endCharToRemove] + // - the third one to preserve, valid range [endCharToRemove+1;len-1] or the empty range if endCharToRemove==len-1 + // NOTE: empty range != range [0;0] since the range [0;0] contains 1 character (the zero-th one)! initialCharToRemove = len/2; - size_t endChar = len/2; + size_t endCharToRemove = len/2; // index of the last character to remove; valid range is [0;len-1] int removedPx = 0; for ( ; removedPx < excessPx; ) { + // try to remove the last character of the first part of the string if (initialCharToRemove > 0) { - // widthPx of the initialCharToRemove-th character - int widthPx = charOffsetsPx[initialCharToRemove] - - charOffsetsPx[initialCharToRemove-1]; - - // remove the initialCharToRemove-th character - removedPx += widthPx; + // width of the (initialCharToRemove-1)-th character + int widthPx; + if (initialCharToRemove >= 2) + widthPx = charOffsetsPx[initialCharToRemove-1] - charOffsetsPx[initialCharToRemove-2]; + else + widthPx = charOffsetsPx[initialCharToRemove-1]; + // the (initialCharToRemove-1)-th character is the first char of the string + + wxASSERT(widthPx >= 0); // widthPx is zero for e.g. tab characters + + // mark the (initialCharToRemove-1)-th character as removable initialCharToRemove--; + removedPx += widthPx; } - if (endChar < len - 1 && + // try to remove the first character of the last part of the string + if (endCharToRemove < len - 1 && removedPx < excessPx) { - // widthPx of the (endChar+1)-th character - int widthPx = charOffsetsPx[endChar+1] - - charOffsetsPx[endChar]; + // width of the (endCharToRemove+1)-th character + int widthPx = charOffsetsPx[endCharToRemove+1] - + charOffsetsPx[endCharToRemove]; + + wxASSERT(widthPx >= 0); // widthPx is zero for e.g. tab characters - // remove the endChar-th character + // mark the (endCharToRemove+1)-th character as removable + endCharToRemove++; removedPx += widthPx; - endChar++; } - if (initialCharToRemove == 0 && endChar == len-1) + if (initialCharToRemove == 0 && endCharToRemove == len-1) { - nCharsToRemove = len+1; + // we need to remove all the characters of the string! break; } } - initialCharToRemove++; - nCharsToRemove = endChar - initialCharToRemove + 1; + nCharsToRemove = endCharToRemove - initialCharToRemove + 1; } break; case wxELLIPSIZE_END: { - wxASSERT(len > 0); - int maxWidthPx = totalWidthPx - excessPx; - for ( initialCharToRemove = 0; - initialCharToRemove < len && charOffsetsPx[initialCharToRemove] < maxWidthPx; - initialCharToRemove++ ) - ; - if (initialCharToRemove == 0) - { - nCharsToRemove = len; - } - else - { - //initialCharToRemove--; // go back one character - nCharsToRemove = len - initialCharToRemove; - } + // go backward from the end of the string toward the start + for ( initialCharToRemove = len-1; + initialCharToRemove > 0 && charOffsetsPx[initialCharToRemove-1] > maxWidthPx; + initialCharToRemove-- ) + ; + nCharsToRemove = len - initialCharToRemove; } break; @@ -356,25 +371,31 @@ wxString wxControlBase::DoEllipsizeSingleLine(const wxString& curLine, const wxD return curLine; } + wxASSERT(initialCharToRemove >= 0 && initialCharToRemove <= len-1); // see valid range for initialCharToRemove above + wxASSERT(nCharsToRemove >= 1 && nCharsToRemove <= len-initialCharToRemove); // see valid range for nCharsToRemove above + + // erase nCharsToRemove characters after initialCharToRemove (included); + // e.g. if we have the string "foobar" (len = 6) + // ^ + // \--- initialCharToRemove = 2 + // and nCharsToRemove = 2, then we get "foar" wxString ret(curLine); - if (nCharsToRemove >= len) - { - // need to remove the entire row! - ret.clear(); - } - else - { - // erase nCharsToRemove characters after initialCharToRemove (included): - ret.erase(initialCharToRemove, nCharsToRemove+1); + ret.erase(initialCharToRemove, nCharsToRemove); - // if there is space for the replacement dots, add them - if (maxFinalWidthPx > replacementWidthPx) - ret.insert(initialCharToRemove, wxELLIPSE_REPLACEMENT); - } + int removedPx; + if (initialCharToRemove >= 1) + removedPx = charOffsetsPx[initialCharToRemove+nCharsToRemove-1] - charOffsetsPx[initialCharToRemove-1]; + else + removedPx = charOffsetsPx[initialCharToRemove+nCharsToRemove-1]; + wxASSERT(removedPx >= excessPx); + + // if there is space for the replacement dots, add them + if ((int)totalWidthPx-removedPx+replacementWidthPx < maxFinalWidthPx) + ret.insert(initialCharToRemove, wxELLIPSE_REPLACEMENT); // if everything was ok, we should have shortened this line // enough to make it fit in maxFinalWidthPx: - wxASSERT(dc.GetTextExtent(ret).GetWidth() < maxFinalWidthPx); + wxASSERT_LEVEL_2(dc.GetTextExtent(ret).GetWidth() <= maxFinalWidthPx); return ret; } diff --git a/tests/Makefile.in b/tests/Makefile.in index 8895dba60c..e951631108 100644 --- a/tests/Makefile.in +++ b/tests/Makefile.in @@ -136,6 +136,7 @@ TEST_GUI_OBJECTS = \ test_gui_size.o \ test_gui_point.o \ test_gui_colour.o \ + test_gui_ellipsization.o \ test_gui_measuring.o \ test_gui_config.o \ test_gui_comboboxtest.o \ @@ -573,6 +574,9 @@ test_gui_point.o: $(srcdir)/geometry/point.cpp $(TEST_GUI_ODEP) test_gui_colour.o: $(srcdir)/graphics/colour.cpp $(TEST_GUI_ODEP) $(CXXC) -c -o $@ $(TEST_GUI_CXXFLAGS) $(srcdir)/graphics/colour.cpp +test_gui_ellipsization.o: $(srcdir)/graphics/ellipsization.cpp $(TEST_GUI_ODEP) + $(CXXC) -c -o $@ $(TEST_GUI_CXXFLAGS) $(srcdir)/graphics/ellipsization.cpp + test_gui_measuring.o: $(srcdir)/graphics/measuring.cpp $(TEST_GUI_ODEP) $(CXXC) -c -o $@ $(TEST_GUI_CXXFLAGS) $(srcdir)/graphics/measuring.cpp diff --git a/tests/graphics/ellipsization.cpp b/tests/graphics/ellipsization.cpp new file mode 100644 index 0000000000..c8d3a22054 --- /dev/null +++ b/tests/graphics/ellipsization.cpp @@ -0,0 +1,78 @@ +/////////////////////////////////////////////////////////////////////////////// +// Name: tests/graphics/ellipsization.cpp +// Purpose: wxControlBase::*Ellipsize* unit test +// Author: Francesco Montorsi +// Created: 2010-03-10 +// RCS-ID: $Id$ +// Copyright: (c) 2010 Francesco Montorsi +/////////////////////////////////////////////////////////////////////////////// + +// ---------------------------------------------------------------------------- +// headers +// ---------------------------------------------------------------------------- + +#include "testprec.h" + +#ifdef __BORLANDC__ + #pragma hdrstop +#endif + +#include "wx/control.h" +#include "wx/dcmemory.h" + +// ---------------------------------------------------------------------------- +// test class +// ---------------------------------------------------------------------------- + +class EllipsizationTestCase : public CppUnit::TestCase +{ +public: + EllipsizationTestCase() { } + +private: + CPPUNIT_TEST_SUITE( EllipsizationTestCase ); + CPPUNIT_TEST( Ellipsize ); + CPPUNIT_TEST_SUITE_END(); + + void Ellipsize(); + + DECLARE_NO_COPY_CLASS(EllipsizationTestCase) +}; + +// register in the unnamed registry so that these tests are run by default +CPPUNIT_TEST_SUITE_REGISTRATION( EllipsizationTestCase ); + +// also include in it's own registry so that these tests can be run alone +CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( EllipsizationTestCase, "EllipsizationTestCase" ); + +void EllipsizationTestCase::Ellipsize() +{ + wxMemoryDC dc; + + wxString stringsToTest[] = + { + "N", ".", "x", "foobar", wxS("\u03B1"), "Another test", "a very very very very very very very long string", + "\xCE\xB1\xCE\xB2\xCE\xB3\xCE\xB4\xCE\xB5\xCE\xB6\xCE\xB7\xCE\xB8\xCE\xB9", + // alpha+beta+gamma+delta+epsilon+zeta+eta+theta+iota + "\t", "\t\t\t\t\t", "a\tstring\twith\ttabs", + "\n", "\n\n\n\n\n", "a\nstring\nwith\nnewlines", + "&", "&&&&&&&", "a&string&with&newlines", + "\t\n&", "a\t\n&string\t\n&with\t\n&many\t\n&chars" + }; + int flagsToTest[] = { 0, wxELLIPSIZE_FLAGS_PROCESS_MNEMONICS, wxELLIPSIZE_FLAGS_EXPAND_TABS, + wxELLIPSIZE_FLAGS_PROCESS_MNEMONICS|wxELLIPSIZE_FLAGS_EXPAND_TABS }; + wxEllipsizeMode modesToTest[] = { wxELLIPSIZE_START, wxELLIPSIZE_MIDDLE, wxELLIPSIZE_END }; + int widthsToTest[] = { 0, 1, 2, 3, 10, 20, 100 }; + + for (unsigned int s=0; s + + diff --git a/tests/test_vc8_test_gui.vcproj b/tests/test_vc8_test_gui.vcproj index ec87e6d203..ecf5fdfa71 100644 --- a/tests/test_vc8_test_gui.vcproj +++ b/tests/test_vc8_test_gui.vcproj @@ -915,6 +915,10 @@ /> + + diff --git a/tests/test_vc9_test_gui.vcproj b/tests/test_vc9_test_gui.vcproj index 7a899aa08d..941eb998ca 100644 --- a/tests/test_vc9_test_gui.vcproj +++ b/tests/test_vc9_test_gui.vcproj @@ -887,6 +887,10 @@ /> + + -- 2.45.2