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 <[email protected]> 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 [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice
