Re: Advise needed for fixing tdf#41253

Hi.

You have given yourself a nice task which is not even an EASYHACK. hope you succeed.

In order to have your patch reviewed you need to submit it to our gerrit system, see
https://wiki.documentfoundation.org/Development/GetInvolved#Prepare_to_submit_patches 
<https://wiki.documentfoundation.org/Development/GetInvolved#Prepare_to_submit_patches>

Rgds
jan I.


> On 6 Nov 2016, at 11:32, Korrawit Pruegsanusak <detective.conan.1412@gmail.com> wrote:
>
> Hello everyone,
>
> I just have some times to look at tdf#41253.
> (https://bugs.documentfoundation.org/41253) But I don't promise I
> could finish it, so I still didn't assign myself into a bug. Anyway,
> let me explain (and ask) about it:
>
> The bug is about importing pptx file which has shadowed text (one of
> the text effects apart from bold, italic, underlined), but LibO can't
> import "shadowing text".
>
> The problem is, in OOXML structure, shadowing is declared in
> <a:outerShdw> element, which is a child of <a:effectLst>. For more
> info, please see http://www.datypic.com/sc/ooxml/e-a_effectLst-1.html
>
> This is the OOXML sample comparing shadowed and non-shadowed text
> (cropped from slide1.xml of an extracted pptx file):
> ---cut---
> <a:p>
>    <a:r>
>        <a:rPr lang="en-US" dirty="0" smtClean="0">
>            <a:effectLst>
>                <a:outerShdw blurRad="38100" dist="38100"
> dir="2700000" algn="tl"><a:srgbClr val="000000"><a:alpha val="43137"/>
>                    </a:srgbClr>
>                </a:outerShdw>
>            </a:effectLst>
>        </a:rPr>
>        <a:t>Text with shadow</a:t>
>    </a:r>
> </a:p>
> <a:p>
>    <a:r>
>        <a:rPr lang="en-US" dirty="0" smtClean="0"/>
>         <a:t>Text without shadow</a:t>
>    </a:r>
>    <a:endParaRPr lang="th-TH" dirty="0"/>
> </a:p>
> ---cut---
>
> And the code parsing <a:effectLst> is still unsupported, see:
> http://opengrok.libreoffice.org/xref/core/oox/source/drawingml/textcharacterpropertiescontext.cxx#101
>
> This is my patch (also attached to this email). It compiled
> successfully, but the shadowing still didn't work:
>
> ---cut---
> diff --git a/oox/inc/drawingml/textcharacterproperties.hxx
> b/oox/inc/drawingml/textcharacterproperties.hxx
> index 055e58f..2dca075 100644
> --- a/oox/inc/drawingml/textcharacterproperties.hxx
> +++ b/oox/inc/drawingml/textcharacterproperties.hxx
> @@ -56,6 +56,7 @@ struct TextCharacterProperties
>     OptValue< bool >    moUnderlineLineFollowText;
>     OptValue< bool >    moUnderlineFillFollowText;
>     FillProperties      maFillProperties;
> +    OptValue< bool >    moCharShadowed;
>
>     std::vector<css::beans::PropertyValue> maTextEffectsProperties;
>
> diff --git a/oox/source/drawingml/textcharacterproperties.cxx
> b/oox/source/drawingml/textcharacterproperties.cxx
> index 48803de..c5e1253 100644
> --- a/oox/source/drawingml/textcharacterproperties.cxx
> +++ b/oox/source/drawingml/textcharacterproperties.cxx
> @@ -151,6 +151,10 @@ void TextCharacterProperties::pushToPropMap(
> PropertyMap& rPropMap, const XmlFil
>         rPropMap.setProperty( PROP_CharUnderlineHasColor, true);
>         rPropMap.setProperty( PROP_CharUnderlineColor,
> maUnderlineColor.getColor( rFilter.getGraphicHelper() ));
>     }
> +
> +    bool bCharShadowed = moCharShadowed.get(false);
> +    SAL_DEBUG(bCharShadowed);
> +    rPropMap.setProperty( PROP_CharShadowed, /*true */bCharShadowed);
> }
>
> void pushToGrabBag( PropertySet& rPropSet, const
> std::vector<PropertyValue>& aVectorOfProperyValues )
> ---cut---
>
> In textcharacterproperties.cxx, it is where the shadowing process
> done. If I change the argument to "true", like this:
>    rPropMap.setProperty( PROP_CharShadowed, true);
> Then all texts will got shadowed. The problem is, bCharShadowed is
> never "true" in current patch, and I couldn't get it to be "true".
>
> ---cut---
> diff --git a/oox/source/drawingml/textcharacterpropertiescontext.cxx
> b/oox/source/drawingml/textcharacterpropertiescontext.cxx
> index 1c62c1d..ef24ab1 100644
> --- a/oox/source/drawingml/textcharacterpropertiescontext.cxx
> +++ b/oox/source/drawingml/textcharacterpropertiescontext.cxx
> @@ -217,6 +223,10 @@ ContextHandlerRef
> TextCharacterPropertiesContext::onCreateContext( sal_Int32 aEl
>                 return new TextEffectsContext( *this, aElementToken,
> mrTextCharacterProperties.maTextEffectsProperties );
>             }
>             break;
> +        case A_TOKEN(outerShdw):
> +            SAL_DEBUG("A_TOKEN now True");
> +            mrTextCharacterProperties.moCharShadowed = true;
> +            break;
>         default:
>             SAL_WARN("oox",
> "TextCharacterPropertiesContext::onCreateContext: unhandled element: "
> << getBaseToken(aElementToken));
>             break;
> ---cut---
>
> I don't know why bCharShadowed is never "true". But I think this
> approach is likely inappropriate because we already have these:
>
> ---cut---
>        // EG_EffectProperties
>        case A_TOKEN( effectDag ):  // CT_EffectContainer 5.1.10.25
>        case A_TOKEN( effectLst ):  // CT_EffectList 5.1.10.26
>        break;
> ---cut---
>
> in textcharacterpropertiescontext.cxx line 99-102, which I think it's
> more correct to start from these codes.
>
> So, should I start over from the line:
>        case A_TOKEN( effectLst ):  // CT_EffectList 5.1.10.26
> or any advise about my patch would be much appreciated.
>
>
> Thank you very much.
> Best Regards,
> -- 
> Korrawit Pruegsanusak
> <tdf41253_WIP.diff>_______________________________________________
> LibreOffice mailing list
> LibreOffice@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/libreoffice