diff options
author | Michael Stahl <mstahl@redhat.com> | 2012-07-04 17:47:22 +0200 |
---|---|---|
committer | Michael Stahl <mstahl@redhat.com> | 2012-07-04 18:06:10 +0200 |
commit | e4fc7e777fbfa600dcfe8914da81e89329279e7b (patch) | |
tree | 3c17633491fbf236692fb9cd3ec05d05673d42a2 /editeng | |
parent | 549cf1ab08fc5ee78730c139781ad53805e4717d (diff) |
refactor handling of double border widths:
Word uses a completely different definition of "width" of a double border
than OOo and ODF: for Word the width is apparently the largest of the 3
component widths, while OOo and ODF define the width as the total with of
all 3 components. The new border implementation in LO 3.4 was apparently
inspired by Word's double border definition, which resulted in
various import filter regressions, see the previous fixes:
36e43b52992735c622833e923faa63774b9e2f76
e2ffb71305c5f085eec6d396651c76d6daee3406
70a6a4d425558340bb49507975343a3e0a1bdde8
These fixes set the ScaleMetrics, which actually seems sub-optimal as
there is a ScaleItemSet function somewhere that apparently re-scales
all items in an itemset, which could undo the fixes.
Also, one of the fixes actually managed to break RTF/DOCX import
of double borders, as that ended up in the same code via the API.
This commit now reverses the change, so that the width of a border is
now always the total with of all components, which is (imho) much more
intutitive, and also leads to a consistent UI where selecting say 3pt
width has predictable results, no matter what the border style.
The border widths are now converted in the Word format import/export
filters (writerfilter and sw/source/filter/ww8), and various tests
were adapted to the new handling.
(cherry picked from commit 2d045cdb69176b280812dda0b813371cf1ac72e2)
Conflicts:
sw/qa/extras/ooxmltok/ooxmltok.cxx
Change-Id: I50456c49b1a298569607e6c88f19f18441348ac3
Diffstat (limited to 'editeng')
-rw-r--r-- | editeng/inc/editeng/borderline.hxx | 4 | ||||
-rw-r--r-- | editeng/qa/items/borderline_test.cxx | 35 | ||||
-rw-r--r-- | editeng/source/items/borderline.cxx | 147 | ||||
-rw-r--r-- | editeng/source/items/frmitems.cxx | 7 |
4 files changed, 172 insertions, 21 deletions
diff --git a/editeng/inc/editeng/borderline.hxx b/editeng/inc/editeng/borderline.hxx index e026fc208410..7d18587e78fb 100644 --- a/editeng/inc/editeng/borderline.hxx +++ b/editeng/inc/editeng/borderline.hxx @@ -52,6 +52,10 @@ namespace editeng { // values from ::com::sun::star::table::BorderLineStyle typedef sal_Int16 SvxBorderStyle; + /// convert border width in twips between Word formats and LO + double EDITENG_DLLPUBLIC ConvertBorderWidthToWord(SvxBorderStyle, double); + double EDITENG_DLLPUBLIC ConvertBorderWidthFromWord(SvxBorderStyle, double); + class EDITENG_DLLPUBLIC SvxBorderLine { protected: diff --git a/editeng/qa/items/borderline_test.cxx b/editeng/qa/items/borderline_test.cxx index 8a1de32f3ed4..f330451cce13 100644 --- a/editeng/qa/items/borderline_test.cxx +++ b/editeng/qa/items/borderline_test.cxx @@ -99,7 +99,10 @@ void BorderLineTest::testGuessWidthDouble() SvxBorderLine line; line.GuessLinesWidths( DOUBLE, TEST_WIDTH, TEST_WIDTH, TEST_WIDTH ); CPPUNIT_ASSERT_EQUAL( DOUBLE, line.GetBorderLineStyle() ); - CPPUNIT_ASSERT_EQUAL( TEST_WIDTH, line.GetWidth() ); + CPPUNIT_ASSERT_EQUAL( TEST_WIDTH, static_cast<long>(line.GetOutWidth()) ); + CPPUNIT_ASSERT_EQUAL( TEST_WIDTH, static_cast<long>(line.GetInWidth()) ); + CPPUNIT_ASSERT_EQUAL( TEST_WIDTH, static_cast<long>(line.GetDistance()) ); + CPPUNIT_ASSERT_EQUAL( 3*TEST_WIDTH, line.GetWidth() ); } void BorderLineTest::testGuessWidthNoMatch() @@ -108,6 +111,9 @@ void BorderLineTest::testGuessWidthNoMatch() line.GuessLinesWidths( DOUBLE, TEST_WIDTH + 1, TEST_WIDTH + 2, TEST_WIDTH + 3 ); CPPUNIT_ASSERT_EQUAL( DOUBLE, line.GetBorderLineStyle() ); + CPPUNIT_ASSERT_EQUAL( TEST_WIDTH+1, static_cast<long>(line.GetOutWidth()) ); + CPPUNIT_ASSERT_EQUAL( TEST_WIDTH+2, static_cast<long>(line.GetInWidth()) ); + CPPUNIT_ASSERT_EQUAL( TEST_WIDTH+3, static_cast<long>(line.GetDistance())); CPPUNIT_ASSERT_EQUAL( long( (3 * TEST_WIDTH) + 6 ), line.GetWidth() ); } @@ -119,7 +125,14 @@ void BorderLineTest::testGuessWidthThinthickSmallgap() THINTHICKSG_IN_WIDTH, THINTHICKSG_DIST_WIDTH ); CPPUNIT_ASSERT_EQUAL( THINTHICK_SMALLGAP, line.GetBorderLineStyle() ); - CPPUNIT_ASSERT_EQUAL( TEST_WIDTH, line.GetWidth() ); + CPPUNIT_ASSERT_EQUAL( THINTHICKSG_OUT_WIDTH, + static_cast<long>(line.GetOutWidth()) ); + CPPUNIT_ASSERT_EQUAL( THINTHICKSG_IN_WIDTH, + static_cast<long>(line.GetInWidth()) ); + CPPUNIT_ASSERT_EQUAL( THINTHICKSG_DIST_WIDTH, + static_cast<long>(line.GetDistance()) ); + CPPUNIT_ASSERT_EQUAL( THINTHICKSG_OUT_WIDTH + THINTHICKSG_IN_WIDTH + + THINTHICKSG_DIST_WIDTH, line.GetWidth() ); } void BorderLineTest::testGuessWidthThinthickLargegap() @@ -130,7 +143,14 @@ void BorderLineTest::testGuessWidthThinthickLargegap() THINTHICKLG_IN_WIDTH, THINTHICKLG_DIST_WIDTH ); CPPUNIT_ASSERT_EQUAL( THINTHICK_LARGEGAP, line.GetBorderLineStyle() ); - CPPUNIT_ASSERT_EQUAL( TEST_WIDTH, line.GetWidth() ); + CPPUNIT_ASSERT_EQUAL( THINTHICKLG_OUT_WIDTH, + static_cast<long>(line.GetOutWidth()) ); + CPPUNIT_ASSERT_EQUAL( THINTHICKLG_IN_WIDTH, + static_cast<long>(line.GetInWidth()) ); + CPPUNIT_ASSERT_EQUAL( THINTHICKLG_DIST_WIDTH, + static_cast<long>(line.GetDistance()) ); + CPPUNIT_ASSERT_EQUAL( THINTHICKLG_OUT_WIDTH + THINTHICKLG_IN_WIDTH + + THINTHICKLG_DIST_WIDTH, line.GetWidth() ); } void BorderLineTest::testGuessWidthNostyleDouble() @@ -141,7 +161,14 @@ void BorderLineTest::testGuessWidthNostyleDouble() THINTHICKLG_IN_WIDTH, THINTHICKLG_DIST_WIDTH ); CPPUNIT_ASSERT_EQUAL( THINTHICK_LARGEGAP, line.GetBorderLineStyle() ); - CPPUNIT_ASSERT_EQUAL( TEST_WIDTH, line.GetWidth() ); + CPPUNIT_ASSERT_EQUAL( THINTHICKLG_OUT_WIDTH, + static_cast<long>(line.GetOutWidth()) ); + CPPUNIT_ASSERT_EQUAL( THINTHICKLG_IN_WIDTH, + static_cast<long>(line.GetInWidth()) ); + CPPUNIT_ASSERT_EQUAL( THINTHICKLG_DIST_WIDTH, + static_cast<long>(line.GetDistance()) ); + CPPUNIT_ASSERT_EQUAL( THINTHICKLG_OUT_WIDTH + THINTHICKLG_IN_WIDTH + + THINTHICKLG_DIST_WIDTH, line.GetWidth() ); } void BorderLineTest::testGuessWidthNostyleSingle() diff --git a/editeng/source/items/borderline.cxx b/editeng/source/items/borderline.cxx index 7470f5a9b0b6..f6e184fd1814 100644 --- a/editeng/source/items/borderline.cxx +++ b/editeng/source/items/borderline.cxx @@ -120,6 +120,127 @@ SvxBorderLine::SvxBorderLine( const Color *pCol, long nWidth, aColor = *pCol; } +static const double THINTHICK_SMALLGAP_line2 = 15.0; +static const double THINTHICK_SMALLGAP_gap = 15.0; +static const double THINTHICK_LARGEGAP_line1 = 30.0; +static const double THINTHICK_LARGEGAP_line2 = 15.0; +static const double THICKTHIN_SMALLGAP_line1 = 15.0; +static const double THICKTHIN_SMALLGAP_gap = 15.0; +static const double THICKTHIN_LARGEGAP_line1 = 15.0; +static const double THICKTHIN_LARGEGAP_line2 = 30.0; +static const double OUTSET_line1 = 15.0; +static const double INSET_line2 = 15.0; + +double +ConvertBorderWidthFromWord(SvxBorderStyle const eStyle, double const fWidth) +{ + switch (eStyle) + { + // Single lines + case SOLID: + case DOTTED: + case DASHED: + return fWidth; + break; + + // Double lines + case DOUBLE: + return fWidth * 3.0; + break; + + case THINTHICK_MEDIUMGAP: + case THICKTHIN_MEDIUMGAP: + case EMBOSSED: + case ENGRAVED: + return fWidth * 2.0; + break; + + case THINTHICK_SMALLGAP: + return fWidth + THINTHICK_SMALLGAP_line2 + THINTHICK_SMALLGAP_gap; + break; + + case THINTHICK_LARGEGAP: + return fWidth + THINTHICK_LARGEGAP_line1 + THINTHICK_LARGEGAP_line2; + break; + + case THICKTHIN_SMALLGAP: + return fWidth + THICKTHIN_SMALLGAP_line1 + THICKTHIN_SMALLGAP_gap; + break; + + case THICKTHIN_LARGEGAP: + return fWidth + THICKTHIN_LARGEGAP_line1 + THICKTHIN_LARGEGAP_line2; + break; + + case OUTSET: + return (fWidth * 2.0) + OUTSET_line1; + break; + + case INSET: + return (fWidth * 2.0) + INSET_line2; + break; + + default: + assert(false); // should only be called for known border style + return 0; + break; + } +} + +double +ConvertBorderWidthToWord(SvxBorderStyle const eStyle, double const fWidth) +{ + switch (eStyle) + { + // Single lines + case SOLID: + case DOTTED: + case DASHED: + return fWidth; + break; + + // Double lines + case DOUBLE: + return fWidth / 3.0; + break; + + case THINTHICK_MEDIUMGAP: + case THICKTHIN_MEDIUMGAP: + case EMBOSSED: + case ENGRAVED: + return fWidth / 2.0; + break; + + case THINTHICK_SMALLGAP: + return fWidth - THINTHICK_SMALLGAP_line2 - THINTHICK_SMALLGAP_gap; + break; + + case THINTHICK_LARGEGAP: + return fWidth - THINTHICK_LARGEGAP_line1 - THINTHICK_LARGEGAP_line2; + break; + + case THICKTHIN_SMALLGAP: + return fWidth - THICKTHIN_SMALLGAP_line1 - THICKTHIN_SMALLGAP_gap; + break; + + case THICKTHIN_LARGEGAP: + return fWidth - THICKTHIN_LARGEGAP_line1 - THICKTHIN_LARGEGAP_line2; + break; + + case OUTSET: + return (fWidth / 2.0) - OUTSET_line1; + break; + + case INSET: + return (fWidth / 2.0) - INSET_line2; + break; + + default: + assert(false); // should only be called for known border style + return 0; + break; + } +} + /** Get the BorderWithImpl object corresponding to the given #nStyle, all the units handled by the resulting object are Twips and the BorderWidthImpl::GetLine1() corresponds to the Outer Line. @@ -147,35 +268,41 @@ BorderWidthImpl SvxBorderLine::getWidthImpl( SvxBorderStyle nStyle ) case DOUBLE: aImpl = BorderWidthImpl( CHANGE_LINE1 | CHANGE_LINE2 | CHANGE_DIST, - 1.0, 1.0, 1.0 ); + // fdo#46112 fdo#38542 fdo#43249: + // non-constant witdths must sum to 1 + 1.0/3.0, 1.0/3.0, 1.0/3.0 ); break; case THINTHICK_SMALLGAP: - aImpl = BorderWidthImpl( CHANGE_LINE1, 1.0, 15.0, 15.0 ); + aImpl = BorderWidthImpl( CHANGE_LINE1, 1.0, + THINTHICK_SMALLGAP_line2, THINTHICK_SMALLGAP_gap ); break; case THINTHICK_MEDIUMGAP: aImpl = BorderWidthImpl( CHANGE_LINE1 | CHANGE_LINE2 | CHANGE_DIST, - 1.0, 0.5, 0.5 ); + 0.5, 0.25, 0.25 ); break; case THINTHICK_LARGEGAP: - aImpl = BorderWidthImpl( CHANGE_DIST, 30.0, 15.0, 1.0 ); + aImpl = BorderWidthImpl( CHANGE_DIST, + THINTHICK_LARGEGAP_line1, THINTHICK_LARGEGAP_line2, 1.0 ); break; case THICKTHIN_SMALLGAP: - aImpl = BorderWidthImpl( CHANGE_LINE2, 15.0, 1.0, 15.0 ); + aImpl = BorderWidthImpl( CHANGE_LINE2, THICKTHIN_SMALLGAP_line1, + 1.0, THICKTHIN_SMALLGAP_gap ); break; case THICKTHIN_MEDIUMGAP: aImpl = BorderWidthImpl( CHANGE_LINE1 | CHANGE_LINE2 | CHANGE_DIST, - 0.5, 1.0, 0.5 ); + 0.25, 0.5, 0.25 ); break; case THICKTHIN_LARGEGAP: - aImpl = BorderWidthImpl( CHANGE_DIST, 15.0, 30.0, 1.0 ); + aImpl = BorderWidthImpl( CHANGE_DIST, THICKTHIN_LARGEGAP_line1, + THICKTHIN_LARGEGAP_line2, 1.0 ); break; // Engraved / Embossed @@ -188,7 +315,7 @@ BorderWidthImpl SvxBorderLine::getWidthImpl( SvxBorderStyle nStyle ) case ENGRAVED: aImpl = BorderWidthImpl( CHANGE_LINE1 | CHANGE_LINE2 | CHANGE_DIST, - 0.5, 0.5, 1.0 ); + 0.25, 0.25, 0.5 ); break; // Inset / Outset @@ -199,13 +326,13 @@ BorderWidthImpl SvxBorderLine::getWidthImpl( SvxBorderStyle nStyle ) case OUTSET: aImpl = BorderWidthImpl( CHANGE_LINE2 | CHANGE_DIST, - 15.0, 1.0, 1.0 ); + OUTSET_line1, 0.5, 0.5 ); break; case INSET: aImpl = BorderWidthImpl( CHANGE_LINE1 | CHANGE_DIST, - 1.0, 15.0, 1.0 ); + 0.5, INSET_line2, 0.5 ); break; } diff --git a/editeng/source/items/frmitems.cxx b/editeng/source/items/frmitems.cxx index fb46de9f783d..da4ca7dba164 100644 --- a/editeng/source/items/frmitems.cxx +++ b/editeng/source/items/frmitems.cxx @@ -1791,13 +1791,6 @@ lcl_lineToSvxLine(const table::BorderLine& rLine, SvxBorderLine& rSvxLine, sal_B sal_uInt16( bConvert ? MM100_TO_TWIP(rLine.InnerLineWidth) : rLine.InnerLineWidth ), sal_uInt16( bConvert ? MM100_TO_TWIP(rLine.LineDistance ) : rLine.LineDistance )); } - else - { - if (DOUBLE == rSvxLine.GetBorderLineStyle()) - { // fdo#46112: divide width by 3 for outer line, gap, inner line - rSvxLine.ScaleMetrics(1, 3); - } - } sal_Bool bRet = !rSvxLine.isEmpty(); return bRet; |