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: [email protected]
>>>> For additional commands, e-mail: [email protected]
>>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: [email protected]
>>> For additional commands, e-mail: [email protected]
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [email protected]
>> For additional commands, e-mail: [email protected]
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
