Jief_Machak Posted March 29, 2020 Author Share Posted March 29, 2020 I made a litteral operator _XS. So now, for litteral, you can do "blabla"_XS instead of XString().takeValueFrom("blabla"). I misunderstood the out parameter in ParseSVGXIcon. So I changed it to XImage*. Less compile errors. 1 Link to comment Share on other sites More sharing options...
Jief_Machak Posted March 29, 2020 Author Share Posted March 29, 2020 VectorGraphics, line 489 & 492 : SelectionImages still refer to the global EG_IMAGE *SelectionImages[6]. Maybe ParseSVGXTheme should become a method of XTheme, so SelectionImages would refer to the member ? Link to comment Share on other sites More sharing options...
Slice Posted March 29, 2020 Share Posted March 29, 2020 2 hours ago, Jief_Machak said: VectorGraphics, line 489 & 492 : SelectionImages still refer to the global EG_IMAGE *SelectionImages[6]. Maybe ParseSVGXTheme should become a method of XTheme, so SelectionImages would refer to the member ? yes it is not thoughtfully to the end Link to comment Share on other sites More sharing options...
Jief_Machak Posted March 29, 2020 Author Share Posted March 29, 2020 work in progress... :-) Link to comment Share on other sites More sharing options...
Slice Posted March 29, 2020 Share Posted March 29, 2020 On 3/25/2020 at 9:42 PM, Jief_Machak said: I've switched to new printf format for DebugLog. So now utf8 and utf16 works. : %s for CHAR8* instead of %a %ls for CHAR16* (wchar_t*). Before, %s was only working if char code were < 128 (useless :-). It may not works in console because of console charset %x output hex letter as lower case. It wasn't the case before. %X works as usual. %lld for 64 bits. If you use GCC or Xcode, there is warnings in case of mismatch. See ~~~ - AsciiSPrint(NameCard, 32, "pci%x,%x\0", FakeVen, FakeID); + snprintf(NameCard, 32, "pci%X,%X", FakeVen, FakeID); LowCase(NameCard); ~~~ Next line is to convert upper letters to lower case letters. But isn't it better to use %x format? Link to comment Share on other sites More sharing options...
Jief_Machak Posted March 29, 2020 Author Share Posted March 29, 2020 %x for Ascii..Print function prints in capital letter. To not change behaviour, I converted %x in %X. Just in case some other part of the code would parse this and assume it is capital letter. But of course, if it's nicer in lowercase letter, you can put it back to %x. I answered a bit fast. I did systematic replacement to not change behaviour. I didn't see that this was converted to lowercase afterward. So definitely YES, we can print directly in lowercase and spare the LowCase(NameCard); Link to comment Share on other sites More sharing options...
Slice Posted March 29, 2020 Share Posted March 29, 2020 1 minute ago, Jief_Machak said: %x for Ascii..Print function prints in capital letter. To not change behaviour, I converted %x in %X. Just in case some other part of the code would parse this and assume it is capital letter. But of course, if it's nicer in lowercase letter, you can put it back to %x. Yes, this is very special case where lower case is obligatory. Link to comment Share on other sites More sharing options...
Jief_Machak Posted March 29, 2020 Author Share Posted March 29, 2020 There is 5 LowCase in the whole project. All of them can be removed by using %x instead of %X. Can do it tomorrow... Link to comment Share on other sites More sharing options...
Slice Posted March 30, 2020 Share Posted March 30, 2020 Why the procedure void XTheme::FillByEmbedded() crashes? 4:841 0:002 XArray::ElementAt(xsize) -> Operator [] : index > m_lenA fatal error happened. System halted Link to comment Share on other sites More sharing options...
Jief_Machak Posted March 30, 2020 Author Share Posted March 30, 2020 I fix XImage assignment operator. Should be better now. 1 Link to comment Share on other sites More sharing options...
Slice Posted March 31, 2020 Share Posted March 31, 2020 Now USE_XTHEME is compilable and the Clover started. It is not working because of logical errors. Link to comment Share on other sites More sharing options...
Jief_Machak Posted March 31, 2020 Author Share Posted March 31, 2020 Please, don't add any AllocateZeroPool(), the goal is to remove them ! (ex: legacy.cpp:151). If you need to convert a String, use, for example in that case XString().takeValueFrom(Volume->LegacyOS->IconName) If you need to give the result to a function that wants CHAR8* : XString().takeValueFrom(Volume->LegacyOS->IconName).c_str() The other way around (utf8 to utf16) XStringW().takeValueFrom("blabla") XStringW().takeValueFrom("blabla").wc_str() The ideal would be to refactor the struct LEGACY_OS and replace "CONST CHAR16 *IconName;" by "XString IconName" or "XStringW IconName" (I think XString is enough). But I totally agree not to refactor everything at the same time and these "takeValueFrom()" are precisely there for that : bridging old code with new. Link to comment Share on other sites More sharing options...
Pene Posted March 31, 2020 Share Posted March 31, 2020 (edited) Hi Jief, I'm getting these warnings lately: Active Platform = CloverBootloader/Clover.dsc ...build: : warning: Module MetaFile [Sources] is missing local header! Local Header: cloverbootloader/refit_uefi/cpp_unit_test/printlib-test.h not found in CloverBootloader/rEFIt_UEFI/refit.inf build: : warning: Module MetaFile [Sources] is missing local header! Local Header: cloverbootloader/refit_uefi/cpp_unit_test/printlib-test.h not found in CloverBootloader/rEFIt_UEFI/refit.inf build: : warning: Module MetaFile [Sources] is missing local header! Local Header: cloverbootloader/refit_uefi/cpp_unit_test/poolprint-test.h not found in CloverBootloader/rEFIt_UEFI/refit.inf build: : warning: Module MetaFile [Sources] is missing local header! Local Header: cloverbootloader/refit_uefi/cpp_unit_test/poolprint-test.h not found in CloverBootloader/rEFIt_UEFI/refit.inf done! I added some missing files to refit.inf previously when similar warnings appeared, but I thought to bring it to your attention to look what actually should be in refit.inf Edited March 31, 2020 by Pene Link to comment Share on other sites More sharing options...
Jief_Machak Posted March 31, 2020 Author Share Posted March 31, 2020 I know you're right. Because I'm not using the edk build system (way too slow) for the dev, I usually do see these warning. I compile once with edk build system before committing. And I don't always see the warnings because they are at the beginning. And sometimes it's quite late at night... :-) My bad, I'll try to get the habit to add it to refint.inf as soon as I create a .h. Link to comment Share on other sites More sharing options...
Slice Posted March 31, 2020 Share Posted March 31, 2020 2 hours ago, Jief_Machak said: Please, don't add any AllocateZeroPool(), the goal is to remove them ! (ex: legacy.cpp:151). If you need to convert a String, use, for example in that case XString().takeValueFrom(Volume->LegacyOS->IconName) If you need to give the result to a function that wants CHAR8* : XString().takeValueFrom(Volume->LegacyOS->IconName).c_str() The other way around (utf8 to utf16) XStringW().takeValueFrom("blabla") XStringW().takeValueFrom("blabla").wc_str() The ideal would be to refactor the struct LEGACY_OS and replace "CONST CHAR16 *IconName;" by "XString IconName" or "XStringW IconName" (I think XString is enough). But I totally agree not to refactor everything at the same time and these "takeValueFrom()" are precisely there for that : bridging old code with new. What about this exercise? CHAR16 *TmpArgs = NULL; if (AsciiStrLen(gSettings.BootArgs) > 0) { TmpArgs = PoolPrint(L"%a", gSettings.BootArgs); } ... gSettings.OptionsBits = EncodeOptions(TmpArgs); ... if (TmpArgs) { FreePool(TmpArgs); TmpArgs = NULL; } It looks like can be smarter. Link to comment Share on other sites More sharing options...
Jief_Machak Posted March 31, 2020 Author Share Posted March 31, 2020 XStringW TmpArgs; if (AsciiStrLen(gSettings.BootArgs) > 0) { TmpArgs.SPrintf("%s", gSettings.BootArgs); } ... gSettings.OptionsBits = EncodeOptions(TmpArgs.wc_str()); ... // if (TmpArgs) { // FreePool(TmpArgs); // TmpArgs = NULL; // } Something like that ? Link to comment Share on other sites More sharing options...
Slice Posted March 31, 2020 Share Posted March 31, 2020 Why not gSettings.OptionsBits = EncodeOptions(XStringW().takeValueFrom(gSettings.BootArgs).wc_str()); Will empty input string to be a problem? Link to comment Share on other sites More sharing options...
Jief_Machak Posted March 31, 2020 Author Share Posted March 31, 2020 1 hour ago, Slice said: Why not gSettings.OptionsBits = EncodeOptions(XStringW().takeValueFrom(gSettings.BootArgs).wc_str()); Will empty input string to be a problem? Actually, it's not. I'm surprised but takeValueFrom call StrCpy that allows NULL value to be interpreted the same as an empty string. I also saw that you modified XStringWP that way. I'm usually not a big fan of this, because it makes takeValueFrom(NULL) and takeValueFrom("") producing the same result. By experience, I can say that it sometime lead to undetected problems. In the other hand, it makes thing easier during the refactoring... A middle solution could be to have a new takeValueFromAllowNULL , so at least, we know it's intentional. I know you don't like long method names but there is 2 big advantages : it's 100% cleat what it does, and it can be tracked down easily. Once we have converted all the strings, calls to method takeValueFrom must naturally disappeared. If not, it's easy to find them. Link to comment Share on other sites More sharing options...
Jief_Machak Posted March 31, 2020 Author Share Posted March 31, 2020 Arg, answer too fast again ! gSettings.BootArgs can't be NULL, just empty. So yes, this works : gSettings.OptionsBits = EncodeOptions(XStringW().takeValueFrom(gSettings.BootArgs).wc_str()); Link to comment Share on other sites More sharing options...
Jief_Machak Posted March 31, 2020 Author Share Posted March 31, 2020 I removed XStringWP. That was temporary anyway. To create an XString from litteral, it's now "blabla"_XS or L"blabla"_XSW. Easiest to initialise a XString : XString str = 'blabla"_XS; XString str = SPrintf(...); // with Return Value Optimization, only one allocation must take place. Same for wide XStringW strw = 'blabla"_XSW; XStringW strw = SWPrintf(...);// with Return Value Optimization, only one allocation must take place. Of course, "blabla"_XS can be given directly as a parameter to a method that accept const XString& printf family is way more efficient than concatenation like "bla1"_XS + str + "bla2"_XS. Link to comment Share on other sites More sharing options...
Slice Posted April 1, 2020 Share Posted April 1, 2020 @Jief_Machak I committed // all_tests(); It is not needed for users while you know where it was. Link to comment Share on other sites More sharing options...
Slice Posted April 2, 2020 Share Posted April 2, 2020 It seems I encounter a problem with XImage SelectionImages[6]; SelectionImages[0] is not the same I created. Looks like I have to unroll the array into separate images. Link to comment Share on other sites More sharing options...
Jief_Machak Posted April 2, 2020 Author Share Posted April 2, 2020 Tell me about the problem. I'll have a look with gdb. Link to comment Share on other sites More sharing options...
Slice Posted April 2, 2020 Share Posted April 2, 2020 Compile with USE_XTHEME and see GUI with theme "cesium". Almost all is good except selection images. Link to comment Share on other sites More sharing options...
Jief_Machak Posted April 2, 2020 Author Share Posted April 2, 2020 On 4/1/2020 at 6:04 PM, Slice said: // all_tests(); Looks like when the tests are working here, they are working with everyone. Couldn't be 100% sure at the beginning, that's why I put the test at the beginning of refit_main. What would be great, is to make a separate efi with all the tests. Some courage anyone ? Link to comment Share on other sites More sharing options...
Recommended Posts