From 157b027c4cfca2582b1c1bdb66992560084ac008 Mon Sep 17 00:00:00 2001 From: Regina Henschel Date: Thu, 13 Jan 2022 13:26:25 +0100 Subject: tdf#145700 Improve lighting in extruded custom shapes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fix tries to make rendering similar to MS Office. The ODF standard follows closely the extrusion in RTF and MS binary format. Rendering uses the 3D scene engine. The main problem was, that the z-component of the direction was interpreted with opposite sign. As result the maximum of a light was at false position. Especially a direction from the observer to the object has produced a light behind the shape and so looks as if light was off. The wrong z-direction has produced lighting, which was less intensive than in MS Office. To compensate that, a third light source was added as workaround. That part is removed. Second problem was wrong use of 3D-scene D3DMaterialSpecularIntensity and D3DMaterialSpecular (= UI Specular color). That was not only wrong in OOo but in my previous patch too. D3DMaterialSpecularIntensity corresponds to MS property 'c3DShininess'. Relationship is Intensity = 2^c3DShininess. D3DMaterialSpecular is calculated from MS property c3DSpecularAmt and and c3DKeyIntensity. The light source was missing, but needs to be included, because c3DSpecularAmt is 'the ratio of incident to specular light that is reflected on a shape'. The old unit tests are adapted to this change. MS gives no information how it softens a light in case of harsh=false. ODF specifies it as 'implementation-defined'. The patch uses four additional lights, which have directions in 60° angle to the original light. The light intensity is distributed. That comes near to rendering in MS Office. Changing our 3D engine to provide 'soft' lights was not doable for me. The way MS Office renders a 'metal' surface is different from ODF specification. To distinguish the kinds, I have introduced a new property MetalType. I have discussed it with the ODF TC (see minutes from 2022-02-07) and got the advise to use namespaced values. Therefore the datatype is not boolean. The 'Surface' drop-down in the extrusion bar is changed to make the two kinds of rendering 'Metal' available to the user. If a user sets surface 'Metal' in the UI of MS Office, it sets not only fc3DMetallic but reduces the value of c3DDiffuseAmt in addition. Our 3D-scene engine has the corresponding ODF attribute dr3d:diffuse-color not implemented. To get a similar rendering I change the material color of the 3D-objects as workaround. Change-Id: Ia986b9c318b4c79688e0c0e2d858215b9d612fdc Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128449 Tested-by: Jenkins Reviewed-by: Regina Henschel --- xmloff/inc/EnhancedCustomShapeToken.hxx | 2 + .../data/tdf145700_3D_metal_type_MSCompatible.doc | Bin 0 -> 27136 bytes xmloff/qa/unit/draw.cxx | 100 +++++++++++++++++++++ xmloff/source/core/xmltoken.cxx | 1 + xmloff/source/draw/EnhancedCustomShapeToken.cxx | 2 + xmloff/source/draw/shapeexport.cxx | 20 +++++ xmloff/source/draw/ximpcustomshape.cxx | 13 +++ xmloff/source/token/tokens.txt | 1 + 8 files changed, 139 insertions(+) create mode 100644 xmloff/qa/unit/data/tdf145700_3D_metal_type_MSCompatible.doc (limited to 'xmloff') diff --git a/xmloff/inc/EnhancedCustomShapeToken.hxx b/xmloff/inc/EnhancedCustomShapeToken.hxx index 328704f9b889..7fd86f5a5126 100644 --- a/xmloff/inc/EnhancedCustomShapeToken.hxx +++ b/xmloff/inc/EnhancedCustomShapeToken.hxx @@ -47,6 +47,7 @@ namespace xmloff::EnhancedCustomShapeToken { EAS_extrusion_first_light_direction, EAS_extrusion_second_light_direction, EAS_extrusion_metal, + EAS_extrusion_metal_type, EAS_shade_mode, EAS_extrusion_rotation_angle, EAS_extrusion_rotation_center, @@ -115,6 +116,7 @@ namespace xmloff::EnhancedCustomShapeToken { EAS_FirstLightDirection, EAS_SecondLightDirection, EAS_Metal, + EAS_MetalType, EAS_ShadeMode, EAS_RotateAngle, EAS_RotationCenter, diff --git a/xmloff/qa/unit/data/tdf145700_3D_metal_type_MSCompatible.doc b/xmloff/qa/unit/data/tdf145700_3D_metal_type_MSCompatible.doc new file mode 100644 index 000000000000..99c433654dcd Binary files /dev/null and b/xmloff/qa/unit/data/tdf145700_3D_metal_type_MSCompatible.doc differ diff --git a/xmloff/qa/unit/draw.cxx b/xmloff/qa/unit/draw.cxx index fc07053cbacf..04c7178ca6bf 100644 --- a/xmloff/qa/unit/draw.cxx +++ b/xmloff/qa/unit/draw.cxx @@ -16,15 +16,19 @@ #include #include #include +#include #include #include #include #include #include +#include +#include #include #include #include +#include using namespace ::com::sun::star; @@ -44,6 +48,7 @@ public: void registerNamespaces(xmlXPathContextPtr& pXmlXpathCtx) override; uno::Reference& getComponent() { return mxComponent; } void save(const OUString& rFilterName, utl::TempFile& rTempFile); + uno::Reference getShape(sal_uInt8 nShapeIndex); }; void XmloffDrawTest::setUp() @@ -76,6 +81,17 @@ void XmloffDrawTest::save(const OUString& rFilterName, utl::TempFile& rTempFile) validate(rTempFile.GetFileName(), test::ODF); } +uno::Reference XmloffDrawTest::getShape(sal_uInt8 nShapeIndex) +{ + uno::Reference xDrawPagesSupplier(mxComponent, + uno::UNO_QUERY_THROW); + uno::Reference xDrawPages(xDrawPagesSupplier->getDrawPages()); + uno::Reference xDrawPage(xDrawPages->getByIndex(0), uno::UNO_QUERY_THROW); + uno::Reference xShape(xDrawPage->getByIndex(nShapeIndex), + uno::UNO_QUERY_THROW); + return xShape; +} + CPPUNIT_TEST_FIXTURE(XmloffDrawTest, testTextBoxLoss) { // Load a document that has a shape with a textbox in it. Save it to ODF and reload. @@ -242,6 +258,90 @@ CPPUNIT_TEST_FIXTURE(XmloffDrawTest, testTableInShape) CPPUNIT_ASSERT_EQUAL(OUString("A1"), xCell->getString()); } +// Tests for save/load of new (LO 7.4) attribute loext:extrusion-metal-type +namespace +{ +void lcl_assertMetalProperties(std::string_view sInfo, uno::Reference& rxShape) +{ + uno::Reference xShapeProps(rxShape, uno::UNO_QUERY); + uno::Sequence aGeoPropSeq; + xShapeProps->getPropertyValue("CustomShapeGeometry") >>= aGeoPropSeq; + comphelper::SequenceAsHashMap aGeoPropMap(aGeoPropSeq); + uno::Sequence aExtrusionSeq; + aGeoPropMap.getValue("Extrusion") >>= aExtrusionSeq; + comphelper::SequenceAsHashMap aExtrusionPropMap(aExtrusionSeq); + + bool bIsMetal(false); + aExtrusionPropMap.getValue("Metal") >>= bIsMetal; + OString sMsg = OString::Concat(sInfo) + " Metal"; + CPPUNIT_ASSERT_MESSAGE(sMsg.getStr(), bIsMetal); + + sal_Int16 nMetalType(-1); + aExtrusionPropMap.getValue("MetalType") >>= nMetalType; + sMsg = OString::Concat(sInfo) + " MetalType"; + CPPUNIT_ASSERT_EQUAL_MESSAGE( + sMsg.getStr(), css::drawing::EnhancedCustomShapeMetalType::MetalMSCompatible, nMetalType); +} +} + +CPPUNIT_TEST_FIXTURE(XmloffDrawTest, testExtrusionMetalTypeExtended) +{ + // import + getComponent() = loadFromDesktop(m_directories.getURLFromSrc(DATA_DIRECTORY) + + "tdf145700_3D_metal_type_MSCompatible.doc", + "com.sun.star.text.TextDocument"); + // verify properties + uno::Reference xShape(getShape(0)); + lcl_assertMetalProperties("from doc", xShape); + + // Test, that new attribute is written with loext namespace. Adapt when attribute is added to ODF. + utl::TempFile aTempFile; + // The file has set c3DSpecularAmt="65536" to prevent validation error in attribute + // draw:extrusion-specularity. The error, that 122% is written in case of c3DSpecularAmt="80000" is + // not yet fixed. + save("writer8", aTempFile); + + // assert XML. + std::unique_ptr pStream = parseExportStream(aTempFile, "content.xml"); + xmlDocUniquePtr pXmlDoc = parseXmlStream(pStream.get()); + assertXPath(pXmlDoc, "//draw:enhanced-geometry", "extrusion-metal", "true"); + assertXPath(pXmlDoc, + "//draw:enhanced-geometry[@loext:extrusion-metal-type='loext:MetalMSCompatible']"); + + // reload + getComponent()->dispose(); + getComponent() = loadFromDesktop(aTempFile.GetURL(), "com.sun.star.text.TextDocument"); + // verify properties + uno::Reference xShapeReload(getShape(0)); + lcl_assertMetalProperties("from ODF 1.3 extended", xShapeReload); +} + +CPPUNIT_TEST_FIXTURE(XmloffDrawTest, testExtrusionMetalTypeStrict) +{ + // import + getComponent() = loadFromDesktop(m_directories.getURLFromSrc(DATA_DIRECTORY) + + "tdf145700_3D_metal_type_MSCompatible.doc", + "com.sun.star.text.TextDocument"); + + // save ODF 1.3 strict and test, that new attribute is not written. Adapt when attribute is + // added to ODF. + const SvtSaveOptions::ODFDefaultVersion nCurrentODFVersion(GetODFDefaultVersion()); + SetODFDefaultVersion(SvtSaveOptions::ODFVER_013); + // The file has set c3DSpecularAmt="65536" to prevent validation error in attribute + // draw:extrusion-specularity. The error, that 122% is written in case of c3DSpecularAmt="80000" is + // not yet fixed. + utl::TempFile aTempFile; + save("writer8", aTempFile); + + // assert XML. + std::unique_ptr pStream = parseExportStream(aTempFile, "content.xml"); + xmlDocUniquePtr pXmlDoc = parseXmlStream(pStream.get()); + assertXPath(pXmlDoc, "//draw:enhanced-geometry", "extrusion-metal", "true"); + assertXPath(pXmlDoc, "//draw:enhanced-geometry[@loext:extrusion-metal-type]", 0); + + SetODFDefaultVersion(nCurrentODFVersion); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/xmloff/source/core/xmltoken.cxx b/xmloff/source/core/xmltoken.cxx index 298dd431a0fe..f3ae86566e43 100644 --- a/xmloff/source/core/xmltoken.cxx +++ b/xmloff/source/core/xmltoken.cxx @@ -2473,6 +2473,7 @@ namespace xmloff::token { TOKEN( "extrusion-first-light-direction" , XML_EXTRUSION_FIRST_LIGHT_DIRECTION ), TOKEN( "extrusion-second-light-direction" , XML_EXTRUSION_SECOND_LIGHT_DIRECTION ), TOKEN( "extrusion-metal" , XML_EXTRUSION_METAL ), + TOKEN( "extrusion-metal-type" , XML_EXTRUSION_METAL_TYPE ), TOKEN( "extrusion-rotation-angle" , XML_EXTRUSION_ROTATION_ANGLE ), TOKEN( "extrusion-rotation-center" , XML_EXTRUSION_ROTATION_CENTER ), TOKEN( "extrusion-shininess" , XML_EXTRUSION_SHININESS ), diff --git a/xmloff/source/draw/EnhancedCustomShapeToken.cxx b/xmloff/source/draw/EnhancedCustomShapeToken.cxx index 38ca0df48e6a..700e29fc71fd 100644 --- a/xmloff/source/draw/EnhancedCustomShapeToken.cxx +++ b/xmloff/source/draw/EnhancedCustomShapeToken.cxx @@ -59,6 +59,7 @@ const TokenTable pTokenTableArray[] = { "extrusion-first-light-direction", EAS_extrusion_first_light_direction }, { "extrusion-second-light-direction", EAS_extrusion_second_light_direction }, { "extrusion-metal", EAS_extrusion_metal }, + { "extrusion-metal-type", EAS_extrusion_metal_type }, { "shade-mode", EAS_shade_mode }, { "extrusion-rotation-angle", EAS_extrusion_rotation_angle }, { "extrusion-rotation-center", EAS_extrusion_rotation_center }, @@ -127,6 +128,7 @@ const TokenTable pTokenTableArray[] = { "FirstLightDirection", EAS_FirstLightDirection }, { "SecondLightDirection", EAS_SecondLightDirection }, { "Metal", EAS_Metal }, + { "MetalType", EAS_MetalType }, { "ShadeMode", EAS_ShadeMode }, { "RotateAngle", EAS_RotateAngle }, { "RotationCenter", EAS_RotationCenter }, diff --git a/xmloff/source/draw/shapeexport.cxx b/xmloff/source/draw/shapeexport.cxx index f1c23d3ed0b2..ea33330994b1 100644 --- a/xmloff/source/draw/shapeexport.cxx +++ b/xmloff/source/draw/shapeexport.cxx @@ -46,6 +46,7 @@ #include #include #include +#include #include #include #include @@ -4453,6 +4454,25 @@ static void ImpExportEnhancedGeometry( SvXMLExport& rExport, const uno::Referenc bExtrusionMetal ? GetXMLToken( XML_TRUE ) : GetXMLToken( XML_FALSE ) ); } break; + case EAS_MetalType : + { + // export only if ODF extensions are enabled + sal_Int16 eMetalType; + if (rProp.Value >>= eMetalType) + { + SvtSaveOptions::ODFSaneDefaultVersion eVersion = rExport.getSaneDefaultVersion(); + if (eVersion > SvtSaveOptions::ODFSVER_013 + && (eVersion & SvtSaveOptions::ODFSVER_EXTENDED)) + { + if (eMetalType == drawing::EnhancedCustomShapeMetalType::MetalMSCompatible) + aStr = "loext:MetalMSCompatible"; + else + aStr = "draw:MetalODF"; + rExport.AddAttribute(XML_NAMESPACE_LO_EXT, XML_EXTRUSION_METAL_TYPE, aStr); + } + } + } + break; case EAS_ShadeMode : { // shadeMode diff --git a/xmloff/source/draw/ximpcustomshape.cxx b/xmloff/source/draw/ximpcustomshape.cxx index c8dbf70ba143..df86901b9635 100644 --- a/xmloff/source/draw/ximpcustomshape.cxx +++ b/xmloff/source/draw/ximpcustomshape.cxx @@ -40,6 +40,7 @@ #include #include #include +#include #include #include #include @@ -982,6 +983,18 @@ void XMLEnhancedCustomShapeContext::startFastElement( case EAS_extrusion_metal : GetBool( maExtrusion, aIter.toView(), EAS_Metal ); break; + case EAS_extrusion_metal_type : + { + OUString rValue = aIter.toString(); + sal_Int16 eMetalType(drawing::EnhancedCustomShapeMetalType::MetalODF); + if (rValue == "loext:MetalMSCompatible") + eMetalType = drawing::EnhancedCustomShapeMetalType::MetalMSCompatible; + beans::PropertyValue aProp; + aProp.Name = EASGet(EAS_MetalType); + aProp.Value <<= eMetalType; + maExtrusion.push_back(aProp); + } + break; case EAS_shade_mode : { drawing::ShadeMode eShadeMode( drawing::ShadeMode_FLAT ); diff --git a/xmloff/source/token/tokens.txt b/xmloff/source/token/tokens.txt index a23326c71f17..f610bfbff47e 100644 --- a/xmloff/source/token/tokens.txt +++ b/xmloff/source/token/tokens.txt @@ -2320,6 +2320,7 @@ extrusion-second-light-level extrusion-first-light-direction extrusion-second-light-direction extrusion-metal +extrusion-metal-type extrusion-rotation-angle extrusion-rotation-center extrusion-shininess -- cgit