openoffice-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Don Lewis <truck...@apache.org>
Subject Re: SvpGlyphPeer::RemovingGlyph() compile error
Date Tue, 06 Feb 2018 03:20:23 GMT
No, I think there are consistency problems with what members of the
object are used for what purpose.

On  5 Feb, Peter Kovacs wrote:
> If you speak for underlying Issues, you might refer to this?
> I have stashed this change in 
> /main/basebmp/inc/basebmp/scanlineformats.hxx and I do not like the 
> existing implementation.
> 
> namespace basebmp {
>    /* Current implementation */
>         namespace Format {
>                 static const sal_Int32 NONE                         = 0;
>                 static const sal_Int32 ONE_BIT_MSB_GREY             = (sal_Int32)0x01;
>                 static const sal_Int32 ONE_BIT_LSB_GREY             = (sal_Int32)0x02;
>                 static const sal_Int32 ONE_BIT_MSB_PAL              = (sal_Int32)0x03;
>                 static const sal_Int32 ONE_BIT_LSB_PAL              = (sal_Int32)0x04;
>                 static const sal_Int32 FOUR_BIT_MSB_GREY            = (sal_Int32)0x05;
>                 static const sal_Int32 FOUR_BIT_LSB_GREY            = (sal_Int32)0x06;
>                 static const sal_Int32 FOUR_BIT_MSB_PAL             = (sal_Int32)0x07;
>                 static const sal_Int32 FOUR_BIT_LSB_PAL             = (sal_Int32)0x08;
>                 static const sal_Int32 EIGHT_BIT_PAL                = (sal_Int32)0x09;
>                 static const sal_Int32 EIGHT_BIT_GREY               = (sal_Int32)0x0A;
>                 static const sal_Int32 SIXTEEN_BIT_LSB_TC_MASK      = (sal_Int32)0x0B;
>                 static const sal_Int32 SIXTEEN_BIT_MSB_TC_MASK      = (sal_Int32)0x0C;
>                 static const sal_Int32 TWENTYFOUR_BIT_TC_MASK       = (sal_Int32)0x0D;
>                 static const sal_Int32 THIRTYTWO_BIT_TC_MASK        = (sal_Int32)0x0E;
>                 static const sal_Int32 THIRTYTWO_BIT_TC_MASK_ARGB   = (sal_Int32)0x0F;
>                 static const sal_Int32 MAX                          = (sal_Int32)0x0F;
> }
> 
>         /* what it should be
>      enum class Format : sal_Int32
>         { NONE                         = 0
>      , ONE_BIT_MSB_GREY             = 0x01
>         , ONE_BIT_LSB_GREY             = 0x02
>         , ONE_BIT_MSB_PAL              = 0x03
>         , ONE_BIT_LSB_PAL              = 0x04
>         , FOUR_BIT_MSB_GREY            = 0x05
>         , FOUR_BIT_LSB_GREY            = 0x06
>         , FOUR_BIT_MSB_PAL             = 0x07
>         , FOUR_BIT_LSB_PAL             = 0x08
>         , EIGHT_BIT_PAL                = 0x09
>         , EIGHT_BIT_GREY               = 0x0A
>         , SIXTEEN_BIT_LSB_TC_MASK      = 0x0B
>         , SIXTEEN_BIT_MSB_TC_MASK      = 0x0C
>         , TWENTYFOUR_BIT_TC_MASK       = 0x0D
>         , THIRTYTWO_BIT_TC_MASK        = 0x0E
>         , THIRTYTWO_BIT_TC_MASK_ARGB   = 0x0F
>         , MAX                          = 0x0F
>         };
>         */
> }
> 
> I do not feel well with the test options.
> The basic Idea I wanted to go with was to write a specific test, compile 
> it on CentOS 6 (where old and new stuff should work.)
> And make sure possible out put is the same (Blackbox testing)
> 
> Locate all Uses would be also necessary in order to check. Thats where I got stuck on
this topic.
> 
> Please note that enumn class is pretty new. So I do not know if it works on CentOS 6.
I just thought I do write the best code for me and worry about Compiler Version issue as a
later step.
> 
> Also another Idea could be to cut out the code and replace it with a general better design.
But then we need to find the user story to it. Which is a lot of digging.
> And I dont think the code is that bad on first glance.
> 
> On 05.02.2018 19:34, Don Lewis wrote:
>> There is at least one other place in this code that uses the
>> rGlyphData.ExtDataRef().meInfo to Format::NONE comparision.
>>
>> I think there are some deeper problems in this code.  Unfortunately
>> I won't have another chance to dig into it until the end of this week.
>>
>> Something else to think about is how to test this code.
>>
>> On  2 Feb, Peter Kovacs wrote:
>>> It does also not compile on gcc 7.xx
>>>
>>> So I did change the code and it compiles
>>>
>>>      if( rGlyphData.ExtDataRef().mpData != NULL )
>>>
>>> But I think it has to be
>>>
>>>      if( rGlyphData.ExtDataRef().mpData != NULL &&
>>> rGlyphData.ExtDataRef().mpData != Format::NONE)
>>>
>>>
>>> You can work around this issue in gcc by setting some flags that allows
>>> the use of Format::NONE directly. But i concider this as not so good.
>>>
>>> All the best
>>> Peter
>>>
>>> On 30.01.2018 21:01, Don Lewis wrote:
>>>> When doing a build with clang 6, which defaults to c++14, I get a
>>>> compile error in SvpGlyphPeer::RemovingGlyph() in
>>>> vcl/unx/headless/svptext.cxx on this line:
>>>>
>>>>       if( rGlyphData.ExtDataRef().mpData != Format::NONE )
>>>>
>>>> This isn't too surprising since Format::NONE is an integer and mpData is
>>>> a pointer.
>>>>
>>>> There are a couple of ways that I thought of fixing this.  One is to
>>>> change the right side of the comparision to NULL, the other is to change
>>>> the left side to use meInfo.
>>>>
>>>> Then I used OpenGrok to dig through the code, and the only assignments
>>>> to meInfo that I found were in main/vcl/unx/generic/gdi/gcach_xpeer.cxx
>>>> and use the values from this enum:
>>>>     enum { INFO_EMPTY=0, INFO_PIXMAP, INFO_XRENDER, INFO_RAWBMP, INFO_MULTISCREEN
};
>>>>
>>>> This makes no sense given the other comparisions to meInfo in
>>>> svptext.cxx and the code in SvpGlyphPeer::GetGlyphBmp() starting on line
>>>> 92 of that file.  There is a call to rServerFont.SetExtended() that gets
>>>> the value of nBmpFormat, but that sets private member mnExtInfo in class
>>>> ServerFont in glyphcache.hxx, whereas meInfo is a struct field where the
>>>> struct is a private member of class GlyphData.
>>>>
>>>> Thoughts?
>>>>
>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
>>>> For additional commands, e-mail: dev-help@openoffice.apache.org
>>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
>>> For additional commands, e-mail: dev-help@openoffice.apache.org
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
>> For additional commands, e-mail: dev-help@openoffice.apache.org
>>
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
> For additional commands, e-mail: dev-help@openoffice.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
For additional commands, e-mail: dev-help@openoffice.apache.org


Mime
View raw message