I see the error I’ve made now - it’s not possible to move Color to vcl as svl 
uses it. 


> On 11 Feb 2018, at 6:10 pm, Chris Sherlock <chris.sherloc...@gmail.com> wrote:
> 
> Hi all, 
> 
> I am trying to unify our color code. We currently have the following that 
> handles color:
> 
> Color - in tools
> BitmapColor - completely seperate and in vcl
> SalColor - a typedef to a sal_uInt32 with a MAKE_SALCOLOR constexpr
> BColor - part of basegfx, takes three doubles and inherits from B3DTuple
> 
> As a start, I have removed the very unhelpful operator Color() from 
> BitmapColor. It’s not clear that the conversion from BitmapColor to Color is 
> being handled by the operator function, so I changed this to GetColor() to 
> make it very clear that the two are not the same. 
> 
> I firmly believe that the Color class should not be in tools, however. The 
> only modules that use this need vcl, and vcl seems to be the perfect module 
> to keep this. Also, BitmapColor, IMO, should really be derived from Color 
> (thus doing away with BitmapColor::GetColor()).
> 
> As a start I’ve submitted a change to gerrit to move Color from tools to vcl:
> 
> https://gerrit.libreoffice.org/#/c/49169/ 
> <https://gerrit.libreoffice.org/#/c/49169/>
> 
> This compiles cleanly on my Linux and OS X system, but gives the following 
> error on Windows in gerrit (I don’t have a Windows system to compile on, 
> unfortunately) - I genuinely don’t know why this is occurring, can anyone 
> help?
> 
>> Creating library 
>> C:/cygwin/home/tdf/lode/jenkins/workspace/lo_gerrit/Config/windows_msc_dbgutil_32/workdir/LinkTarget/Library/isvl.lib
>>  and object 
>> C:/cygwin/home/tdf/lode/jenkins/workspace/lo_gerrit/Config/windows_msc_dbgutil_32/workdir/LinkTarget/Library/isvl.exp
>> numfmuno.o : error LNK2019: unresolved external symbol 
>> "__declspec(dllimport) public: unsigned long __thiscall 
>> Color::GetColor(void)const " (__imp_?GetColor@Color@@QBEKXZ) referenced in 
>> function "public: virtual long __cdecl 
>> SvNumberFormatterServiceObj::queryColorForNumber(long,double,long)" 
>> (?queryColorForNumber@SvNumberFormatterServiceObj@@UAAJJNJ@Z)
>> zformat.o : error LNK2019: unresolved external symbol "__declspec(dllimport) 
>> public: bool __thiscall Color::operator==(class Color const &)const " 
>> (__imp_??8Color@@QBE_NABV0@@Z) referenced in function "public: void 
>> __thiscall SvNumberformat::GetFormatSpecialInfo(bool &,bool &,unsigned short 
>> &,unsigned short &)const " 
>> (?GetFormatSpecialInfo@SvNumberformat@@QBEXAA_N0AAG1@Z)
>> zforscan.o : error LNK2019: unresolved external symbol 
>> "__declspec(dllimport) public: __thiscall Color::Color(unsigned long)" 
>> (__imp_??0Color@@QAE@K@Z) referenced in function "public: __thiscall 
>> ImpSvNumberformatScan::ImpSvNumberformatScan(class SvNumberFormatter *)" 
>> (??0ImpSvNumberformatScan@@QAE@PAVSvNumberFormatter@@@Z)
>> zforscan.o : error LNK2019: unresolved external symbol 
>> "__declspec(dllimport) public: virtual __thiscall Color::~Color(void)" 
>> (__imp_??1Color@@UAE@XZ) referenced in function "public: __thiscall 
>> ImpSvNumberformatScan::ImpSvNumberformatScan(class SvNumberFormatter *)" 
>> (??0ImpSvNumberformatScan@@QAE@PAVSvNumberFormatter@@@Z)
>> zforscan.o : error LNK2019: unresolved external symbol 
>> "__declspec(dllimport) public: __thiscall Color::Color(class Color const &)" 
>> (__imp_??0Color@@QAE@ABV0@@Z) referenced in function "public: void 
>> __thiscall std::allocator<class Color>::construct<class Color,class 
>> Color>(class Color *,class Color &&)" 
>> (??$construct@VColor@@V1@@?$allocator@VColor@@@std@@QAEXPAVColor@@$$QAV2@@Z)
>> zforscan.o : error LNK2001: unresolved external symbol "public: virtual 
>> unsigned char __thiscall Color::GetBlue(void)const " (?GetBlue@Color@@UBEEXZ)
>> zforscan.o : error LNK2001: unresolved external symbol "public: virtual 
>> unsigned char __thiscall Color::GetGreen(void)const " 
>> (?GetGreen@Color@@UBEEXZ)
>> zforscan.o : error LNK2001: unresolved external symbol "public: virtual 
>> unsigned char __thiscall Color::GetRed(void)const " (?GetRed@Color@@UBEEXZ)
>> zforscan.o : error LNK2001: unresolved external symbol "public: virtual void 
>> __thiscall Color::SetBlue(unsigned char)" (?SetBlue@Color@@UAEXE@Z)
>> zforscan.o : error LNK2001: unresolved external symbol "public: virtual void 
>> __thiscall Color::SetGreen(unsigned char)" (?SetGreen@Color@@UAEXE@Z)
>> zforscan.o : error LNK2001: unresolved external symbol "public: virtual void 
>> __thiscall Color::SetRed(unsigned char)" (?SetRed@Color@@UAEXE@Z)
>> C:/cygwin/home/tdf/lode/jenkins/workspace/lo_gerrit/Config/windows_msc_dbgutil_32/instdir/program/svllo.dll
>>  : fatal error LNK1120: 11 unresolved externals
>> [build CXX] framework/source/helper/statusindicator.cxx
>> [build CXX] framework/source/helper/statusindicatorfactory.cxx
>> [build CXX] framework/source/helper/tagwindowasmodified.cxx
>> [build CXX] framework/source/helper/titlebarupdate.cxx
>> [build CXX] framework/source/helper/uiconfigelementwrapperbase.cxx
>> 
>> mt.exe : general error c101008d: Failed to write the updated manifest to the 
>> resource of file 
>> "C:/cygwin/home/tdf/lode/jenkins/workspace/lo_gerrit/Config/windows_msc_dbgutil_32/instdir/program/svllo.dll".
>>  The system cannot find the file specified.
>> 
>> make[2]: *** 
>> [C:/cygwin/home/tdf/lode/jenkins/workspace/lo_gerrit/Config/windows_msc_dbgutil_32/svl/Library_svl.mk:20:
>>  
>> C:/cygwin/home/tdf/lode/jenkins/workspace/lo_gerrit/Config/windows_msc_dbgutil_32/instdir/program/svllo.dll]
>>  Error 96
>> make[2]: *** Waiting for unfinished jobs....
>> make[2]: Leaving directory 
>> 'C:/cygwin/home/tdf/lode/jenkins/workspace/lo_gerrit/Config/windows_msc_dbgutil_32'
>> make[1]: *** [Makefile:280: build] Error 2
>> make[1]: Leaving directory 
>> 'C:/cygwin/home/tdf/lode/jenkins/workspace/lo_gerrit/Config/windows_msc_dbgutil_32'
>> make: *** [C:/cygwin//home/tdf/lode/bin/cygwrapper.Makefile:6: all] Error 2
>> Build step 'Execute shell' marked build as failure
>> Finished: FAILURE
> 
> 
> ------
> 
> Secondly, I have a gerrit patch that makes BitmapColor inherit from Color:
> 
> https://gerrit.libreoffice.org/#/c/49203/ 
> <https://gerrit.libreoffice.org/#/c/49203/>
> 
> Moving forward, I’d like to get rid of BitmapColor entirely. The following 
> comment was Tomaz, haven’t had a chance to hop onto IRC so I thought I’d send 
> an email to the mailing list. But his comment was:
> 
>> Hi Chris! 
>> 
>> Good work with this but we need to think about what we want to do with Color 
>> first before we make any move like this. Generally, I feel the ultimate 
>> place where Color should actually be is in basegfx, but not in its current 
>> state. There is also BitmapColor in vcl, which does very similar things than 
>> Color and SalColor typedef, then there is also BColor in basegfx, which is a 
>> different beast (double colors channels are an overkill for most practical 
>> purposes) and let's ignore that one, for now at least.
>> 
>> Generally I would like to first combine Color, BitmapColor and SalColor in a 
>> way first. Maybe get rid of BitmapColor altogether as it has very little 
>> purpose. And just have a simplistic ColorData/SalColor like struct or just a 
>> typedef, and then a Color wrapper around it, which does what currently 
>> Color, BitmapColor do. 
>> 
>> There is also that that I'm refactoring those things to add alpha support 
>> (see feature/nativealpha branch), however I don't have much free time to 
>> work on this and there is also that I'm not too confident that this won't 
>> cause regressions. Especially regarding the SVM - StarView Metafile which is 
>> just writing of the Metafile to the disk. This is used in a lot of places 
>> and just changing one structure could possibly lead to breaking of that.
>> 
>> This is why my new effort here is to separate and make it completely 
>> independent, how we store a Metafile to the disk as SVM. So it would become 
>> similar to any other vector format. But again, to do this there need to test 
>> for the Metafile. This is where I started to build svm tests [1], but they 
>> aren't complete. Once they are complete I can separate Metafile file 
>> persisting, which liberates Metafile in a way that it can be changed 
>> independently to the SVM file itself, which then means I can freely also 
>> change structures in VCL and can refactor things more confidently when 
>> adding native alpha support. What a rabbit hole…
>> 
>> Anyway, if you want to join the effort, we could schedule a meeting (on IRC 
>> or whatever is good for you) and I'll explain the ideas I have and how/what 
>> to change if you wish. I'm currently in New Zealand so the timezone 
>> shouldn't be a problem. ;)
>> 
>> [1] 
>> https://cgit.freedesktop.org/libreoffice/core/tree/vcl/qa/cppunit/svm/svmtest.cxx
>>  
>> <https://cgit.freedesktop.org/libreoffice/core/tree/vcl/qa/cppunit/svm/svmtest.cxx>
> Any comments from others would be nice. 
> 
> Thanks,
> Chris
> 

_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice

Reply via email to