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-unsubscr...@openoffice.apache.org >>>> For additional commands, e-mail: dev-h...@openoffice.apache.org >>>> >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: dev-unsubscr...@openoffice.apache.org >>> For additional commands, e-mail: dev-h...@openoffice.apache.org >>> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@openoffice.apache.org >> For additional commands, e-mail: dev-h...@openoffice.apache.org >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@openoffice.apache.org > For additional commands, e-mail: dev-h...@openoffice.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@openoffice.apache.org For additional commands, e-mail: dev-h...@openoffice.apache.org