Jump to content

C++ proposition

Jief_Machak

By Jief_Machak,
in Clover Development

 Share
823 posts in this topic

Recommended Posts

Jief_Machak
Jief_Machak
Posted
  • Author
Posted (edited)

Can I reproduce the problem in Qemu ? Is it possible to add a "fake" partition to Qemu/disk_image_gpt.img with just what is needed for Clover to detect the partition.

I'm in a really big rush on a lot of other things here. If you could do that for me, I'll, then, debug. It'll be easy with a breakpoint to see what's going on.

Edited by Jief_Machak
Link to comment
Share on other sites
Pene
Pene
Posted
Posted
7 minutes ago, Jief_Machak said:

Can I reproduce the problem in Qemu ? Is it possible to add a "fake" partition to Qemu/disk_image_gpt.img with just what is needed for Clover to detect the partition.

I'm in a really big rush on a lot of other things here. If you could do that for me, I'll, then, debug. It'll be easy with a breakpoint to see what's going on.

Yeah, I'm trying that myself actually at the moment. But it hangs somewhere. I'll update when I figure out how to get it working in Qemu.

Link to comment
Share on other sites
Pene
Pene
Posted
Posted (edited)
1 hour ago, Jief_Machak said:

Can I reproduce the problem in Qemu ? Is it possible to add a "fake" partition to Qemu/disk_image_gpt.img with just what is needed for Clover to detect the partition.

I'm in a really big rush on a lot of other things here. If you could do that for me, I'll, then, debug. It'll be easy with a breakpoint to see what's going on.

OK, fixed the hang - it was due to double free of FileInfo (committed that).

So to reproduce the previous bug, from your OSX partition in qemu: 

mkdir boot
echo something > boot/vmlinuz

It is enough to make LinuxScan find it.

 

Also add the loadoptions debug from before at the beginning of main.c@StartLoader(), to see what it boots with: 

DebugLog(2, "LoadOptions at StartLoader: <%s>\n", LoadOptions.ConcatAll(" "_XS8).wc_str()); gBS->Stall(10000000);

 

Then press "space" on the vmlinuz created entry, and choose, for example, the "without splash" option. You will see "splash" is still there when it boots.

Edited by Pene
Link to comment
Share on other sites
Pene
Pene
Posted
Posted (edited)

Seems that the bug is more generic - happens also with Windows entries for example. LoadOptions seems not to be taken from the chosen option, but from the main entry. 

 

The error is related to the code at gui/REFIT_MENU_SCREEN.cpp, here: 

      while (!SubMenuExit) {
        //running details menu
        SubMenuExit = MainChosenEntry->SubScreen->RunGenericMenu(Style, &SubMenuIndex, &TempChosenEntry);
        if ( MainChosenEntry->getREFIT_MENU_ITEM_BOOTNUM() ) DecodeOptions(MainChosenEntry->getREFIT_MENU_ITEM_BOOTNUM());
...
       if (MainChosenEntry->getREFIT_MENU_ENTRY_CLOVER()) {
          MainChosenEntry->getREFIT_MENU_ENTRY_CLOVER()->LoadOptions = (((REFIT_MENU_ENTRY_CLOVER*)TempChosenEntry)->LoadOptions);
        }
...
        if (/*MenuExit == MENU_EXIT_ENTER &&*/ MainChosenEntry->getLOADER_ENTRY()) {
          if (MainChosenEntry->getLOADER_ENTRY()->LoadOptions.notEmpty()) {
            snprintf(gSettings.BootArgs, 255, "%s", MainChosenEntry->getLOADER_ENTRY()->LoadOptions.ConcatAll(" "_XS8).c_str());
          } else {
            ZeroMem(&gSettings.BootArgs, 255);
          }
          DBG(" boot with args: %s\n", gSettings.BootArgs);

The entry with the updated options is in TempChosenEntry.

But we are working with MainChosenEntry. That's why the old options remain, so basically what we updated in the partiallyDuplicatedEntry->LoadOptions is not used.

 

It seems that the options in TempChosenEntry are taken only when MainChosenEntry->getREFIT_MENU_ENTRY_CLOVER() ? 

 

There are many cases there though, not sure how to fix it.

Edited by Pene
Link to comment
Share on other sites
Pene
Pene
Posted
Posted (edited)
11 minutes ago, Jief_Machak said:

Sorry, not sure either. Is it a bug recently introduced ?

Probably not, It was very long when I checked it, but it did work properly once.

I thought at first that it is related to XArray, but it isn't.

I'll try later to figure out what's happening in that code and what cases it is trying to handle, never looked there before.

Edited by Pene
Link to comment
Share on other sites
Slice
Slice
Posted
Posted
11 hours ago, Jief_Machak said:

So, posix implementation can work :). Committed.

But I didn't know about "GuidLEToStr" at that time, I imagine.

Are they doing the same ? If yes, we should keep strguid, not because I did it, but because it avoids memory reallocation.

OK, now strguid works. Yes GuidLEToStr do the same works and we may exclude it replacing by strguid().

But with test all occurrence.

Link to comment
Share on other sites
Pene
Pene
Posted
Posted (edited)

Well as I couldn't find any logical reasoning to why not TempOptions is used when exiting a subentry, I believe it is a mistake, so I committed changes in some places from MainOptions to TempOptions.

According to my tests, all is working fine now.

 

@Slice if you know of any reason for that, and consider it wasn't a mistake, please say.

Edited by Pene
Link to comment
Share on other sites
Slice
Slice
Posted
Posted
8 minutes ago, Pene said:

Well as I couldn't find any logical reasoning to why not TempOptions is used when exiting a subentry, I believe it is a mistake, so I committed changes in some places from MainOptions to TempOptions.

According to my tests, all is working fine now.

Did you notice here null pointer dereferencing? line2595. 

  while (!MenuExit) {
    GetAnime();
    DBG("AnimeRun=%d\n", FilmC->AnimeRun?1:0);

 

Link to comment
Share on other sites
Pene
Pene
Posted
Posted (edited)
2 hours ago, Slice said:

Did you notice here null pointer dereferencing? line2595. 


  while (!MenuExit) {
    GetAnime();
    DBG("AnimeRun=%d\n", FilmC->AnimeRun?1:0);

 

I'm not sure I understand: you mean there is null pointer dereferencing after recent commits?

If yes, did you do anything special to trigger the problem?

Edited by Pene
Link to comment
Share on other sites
Slice
Slice
Posted
Posted
8 hours ago, Pene said:

I'm not sure I understand: you mean there is null pointer dereferencing after recent commits?

If yes, did you do anything special to trigger the problem?

Not recent. It is a part you working now. I committed.

Link to comment
Share on other sites
Slice
Slice
Posted
Posted

Hi @Jief_Machak

VS compilation failed again 

c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(359): error C2027: ᯮ짮 । ⨯ "_xstringarray__char_type<Type1,void>"
        with
        [
            Type1=LString8
        ]
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(359): note:  .  "_xstringarray__char_type<Type1,void>"
        with
        [
            Type1=LString8
        ]
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\XStringArray_test.cpp(55): note: 믮  뫪   蠡 㭪樨 "XStringArrayClass Split<XStringArray,LString8,LString8,void>(Type1,const Type2)"
        with
        [
            XStringArrayClass=XStringArray,
            Type1=LString8,
            Type2=LString8
        ]
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(359): error C3861: getCharPtr: 䨪
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(360): error C2027: ᯮ짮 । ⨯ "_xstringarray__char_type<Type1,void>"
        with
        [
            Type1=LString8

        ]
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(359): note:  .  "_xstringarray__char_type<Type1,void>"
        with
        [
            Type1=LString8
        ]
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(360): error C3861: getCharPtr: 䨪
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(363): error C2668: length_of_utf_string:  맮 ॣ㦥 㭪樨
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\unicode_conversions.h(253): note:   "size_t length_of_utf_string(const wchar_t *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\unicode_conversions.h(252): note:        "size_t length_of_utf_string(const char32_t *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\unicode_conversions.h(251): note:        "size_t length_of_utf_string(const char16_t *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\unicode_conversions.h(250): note:        "size_t length_of_utf_string(const char *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(363): note:  ⪥ ᮯ⠢ ᯨ᮪ 㬥⮢ "(unknown-type)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(379): error C2672: "XStringAbstract__ncompare":   ᮮ⢥ ॣ㦥 㭪
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(379): error C2784: int XStringAbstract__ncompare(const S *,const O *,size_t,bool):  㤠 뢥 㬥 蠡  "const S *"  "unknown-type"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\XStringAbstract.h(112): note:  .  "XStringAbstract__ncompare"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(381): error C2668: get_char32_from_string:  맮 ॣ㦥 㭪樨
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\unicode_conversions.h(234): note:   "const wchar_t *get_char32_from_string(const wchar_t *,char32_t *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\unicode_conversions.h(233): note:        "const char32_t *get_char32_from_string(const char32_t *,char32_t *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\unicode_conversions.h(232): note:        "const char16_t *get_char32_from_string(const char16_t *,char32_t *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\unicode_conversions.h(231): note:        "const char *get_char32_from_string(const char *,char32_t *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(381): note:  ⪥ ᮯ⠢ ᯨ᮪ 㬥⮢ "(unknown-type, char32_t *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(388): error C2672: "XStringAbstract__ncompare":   ᮮ⢥ ॣ㦥 㭪
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(388): error C2784: int XStringAbstract__ncompare(const S *,const O *,size_t,bool):  㤠 뢥 㬥 蠡  "const S *"  "unknown-type"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\XStringAbstract.h(112): note:  .  "XStringAbstract__ncompare"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(390): error C2668: get_char32_from_string:  맮 ॣ㦥 㭪樨
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\unicode_conversions.h(234): note:   "const wchar_t *get_char32_from_string(const wchar_t *,char32_t *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\unicode_conversions.h(233): note:        "const char32_t *get_char32_from_string(const char32_t *,char32_t *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\unicode_conversions.h(232): note:        "const char16_t *get_char32_from_string(const char16_t *,char32_t *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\unicode_conversions.h(231): note:        "const char *get_char32_from_string(const char *,char32_t *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(390): note:  ⪥ ᮯ⠢ ᯨ᮪ 㬥⮢ "(unknown-type, char32_t *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(394): error C2672: "XStringAbstract<char,XString8>::strncpy":   ᮮ⢥ ॣ㦥 㭪
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(394): error C2784: void XStringAbstract<char,XString8>::strncpy(const O *,size_t):  㤠 뢥 㬥 蠡  "const O *"  "unknown-type"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\XStringAbstract.h(813): note:  .  "XStringAbstract<char,XString8>::strncpy"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(399): error C2668: get_char32_from_string:  맮 ॣ㦥 㭪樨
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\unicode_conversions.h(234): note:   "const wchar_t *get_char32_from_string(const wchar_t *,char32_t *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\unicode_conversions.h(233): note:        "const char32_t *get_char32_from_string(const char32_t *,char32_t *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\unicode_conversions.h(232): note:        "const char16_t *get_char32_from_string(const char16_t *,char32_t *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_foundation\unicode_conversions.h(231): note:        "const char *get_char32_from_string(const char *,char32_t *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(399): note:  ⪥ ᮯ⠢ ᯨ᮪ 㬥⮢ "(unknown-type, char32_t *)"
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(359): error C2027: ᯮ짮 । ⨯ "_xstringarray__char_type<Type1,void>"
        with
        [
            Type1=XString8
        ]
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\../cpp_foundation/XStringArray.h(359): note:  .  "_xstringarray__char_type<Type1,void>"
        with
        [
            Type1=XString8
        ]
c:\users\sergey\desktop\work\clover\rEFIt_UEFI\cpp_unit_test\XStringArray_test.cpp(114): note: 믮  뫪   蠡 㭪樨 "XStringArrayClass Split<XStringArray,XString8,const char*,void>(Type1,const Type2)"
        with
        [
            XStringArrayClass=XStringArray,
            Type1=XString8,
            Type2=const char *
        ]
[CPP] AutoGen
        "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.25.28610\bin\Hostx86\x64\lib.exe" /NOLOGO /LTCG /OUT:c:\users\sergey\desktop\work\clover\Build\Clover\RELEASE_VS2017\X64\CloverEFI\AcpiReset\Reset\OUTPUT\AcpiReset.lib @c:\users\sergey\desktop\work\clover\Build\Clover\RELEASE_VS2017\X64\CloverEFI\AcpiReset\Reset\OUTPUT\object_files.lst
NMAKE : fatal error U1077: "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.25.28610\bin\Hostx86\x64\cl.exe" :   "0x2"
Stop.


build.py...
 : error 7000: Failed to execute command
        C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.25.28610\bin\Hostx86\x86\nmake.exe /nologo tbuild [c:\users\sergey\desktop\work\clover\Build\Clover\RELEASE_VS2017\X64\rEFIt_UEFI\refit]


build.py...
 : error F002: Failed to build module
        c:\users\sergey\desktop\work\clover\rEFIt_UEFI\refit.inf [X64, VS2017, RELEASE]

- Failed -
Build end time: 13:35:07, May.14 2020
Build total time: 00:00:29


!!! Error while building !!!

C:\Users\Sergey\Desktop\Work\Clover>

I don't know what is the chinese symbols so it will be better you check this by yourself.

Link to comment
Share on other sites
Slice
Slice
Posted
Posted
On 2/18/2020 at 3:42 PM, apianti said:

 

That is not how you should have fixed that loop. 


UINTN i = 7;
while (i > 0) {
  --i;
  ...
}

What is up with no one knowing the difference between --i/++i and i--/i++? You should almost never be using i--/i++ unless you need the value before decrement/increment in the current expression. Otherwise, this emits at least two more instructions than the prefix operator does.

 

 

No differences.

Source is 

  for (int i = 0; i < 5; i++) {
    j = i * 3;
    printf("%d\n", j);
  }
  for (int i = 0; i < 5; ++i) {
    j = i * 5;
    printf("%d\n", j);
  }

Compilation is 

0000000100000f0d	movl	$0x0, -0x18(%rbp)
0000000100000f14	cmpl	$0x5, -0x18(%rbp)
0000000100000f18	jge	0x100000f47
0000000100000f1e	imull	$0x3, -0x18(%rbp), %eax
0000000100000f22	movl	%eax, -0x14(%rbp)
0000000100000f25	movl	-0x14(%rbp), %esi
0000000100000f28	leaq	0x7b(%rip), %rdi
0000000100000f2f	movb	$0x0, %al
0000000100000f31	callq	0x100000f8a
0000000100000f36	movl	%eax, -0x20(%rbp)
0000000100000f39	movl	-0x18(%rbp), %eax
0000000100000f3c	addl	$0x1, %eax
0000000100000f3f	movl	%eax, -0x18(%rbp)
0000000100000f42	jmp	0x100000f14

0000000100000f47	movl	$0x0, -0x1c(%rbp)
0000000100000f4e	cmpl	$0x5, -0x1c(%rbp)
0000000100000f52	jge	0x100000f81
0000000100000f58	imull	$0x5, -0x1c(%rbp), %eax
0000000100000f5c	movl	%eax, -0x14(%rbp)
0000000100000f5f	movl	-0x14(%rbp), %esi
0000000100000f62	leaq	0x41(%rip), %rdi
0000000100000f69	movb	$0x0, %al
0000000100000f6b	callq	0x100000f8a
0000000100000f70	movl	%eax, -0x24(%rbp)
0000000100000f73	movl	-0x1c(%rbp), %eax
0000000100000f76	addl	$0x1, %eax
0000000100000f79	movl	%eax, -0x1c(%rbp)
0000000100000f7c	jmp	0x100000f4e

Thus we see i++ and ++i are exactly the same when using in the FOR() construction.

Link to comment
Share on other sites
Slice
Slice
Posted
Posted

Hi @Jief_Machak,

Can you help me with memory leak?

See procedure

const XImage& XIcon::GetBest(bool night) const

It created new image every time when called in SVG theme.

But I don't know how to free this memory.

Link to comment
Share on other sites
Jief_Machak
Jief_Machak
Posted
  • Author
Posted

Ok, I see. GetBest return sometimes a newly allocated image, and sometime an already allocated image (Image or ImageNight).

One possibility is that GetBest return an XImage (not an XImage *). But of course, when imageSVG is NULL, that'll create a copy when it's not needed.

Other solution is to rasterize imageSVG in a "cache", which would be a member "XImage ImageSVGRaster" for example.

Then, GetBest can return a const XImage&.

 

PS : I think we should take the time to make a XIconSVG class and XIconPNG with a base class XIcon.

Then, do the same with XTheme.

Things will become more obvious and clearer that way.

I can help with that as I'll have some free time soon (I know it's been a while I'm saying that :( ).

Link to comment
Share on other sites
Jief_Machak
Jief_Machak
Posted
  • Author
Posted

There is also the solution to create a LoadSVGImage. LoadSVGImage will do what GetBest currently does : the rasterization of ImageSVG and ImageSVGNight. ImageSVG and ImageSVGNight members could be then removed from XIcon.

 

In that later case, no need for XIconSVG and XIconPNG because once loaded, there will be no difference.

Link to comment
Share on other sites
apianti
apianti
Posted
Posted
On 5/18/2020 at 3:00 AM, Slice said:

No differences.

Source is 


  for (int i = 0; i < 5; i++) {
    j = i * 3;
    printf("%d\n", j);
  }
  for (int i = 0; i < 5; ++i) {
    j = i * 5;
    printf("%d\n", j);
  }

Compilation is 


0000000100000f0d	movl	$0x0, -0x18(%rbp)
0000000100000f14	cmpl	$0x5, -0x18(%rbp)
0000000100000f18	jge	0x100000f47
0000000100000f1e	imull	$0x3, -0x18(%rbp), %eax
0000000100000f22	movl	%eax, -0x14(%rbp)
0000000100000f25	movl	-0x14(%rbp), %esi
0000000100000f28	leaq	0x7b(%rip), %rdi
0000000100000f2f	movb	$0x0, %al
0000000100000f31	callq	0x100000f8a
0000000100000f36	movl	%eax, -0x20(%rbp)
0000000100000f39	movl	-0x18(%rbp), %eax
0000000100000f3c	addl	$0x1, %eax
0000000100000f3f	movl	%eax, -0x18(%rbp)
0000000100000f42	jmp	0x100000f14

0000000100000f47	movl	$0x0, -0x1c(%rbp)
0000000100000f4e	cmpl	$0x5, -0x1c(%rbp)
0000000100000f52	jge	0x100000f81
0000000100000f58	imull	$0x5, -0x1c(%rbp), %eax
0000000100000f5c	movl	%eax, -0x14(%rbp)
0000000100000f5f	movl	-0x14(%rbp), %esi
0000000100000f62	leaq	0x41(%rip), %rdi
0000000100000f69	movb	$0x0, %al
0000000100000f6b	callq	0x100000f8a
0000000100000f70	movl	%eax, -0x24(%rbp)
0000000100000f73	movl	-0x1c(%rbp), %eax
0000000100000f76	addl	$0x1, %eax
0000000100000f79	movl	%eax, -0x1c(%rbp)
0000000100000f7c	jmp	0x100000f4e

Thus we see i++ and ++i are exactly the same when using in the FOR() construction.

 

You missed my point as it has nothing to do with the loop but with the operators. Also, this is extremely optimized code, my guess is you have it set to the highest level of optimization in a release build. This is part of the issue of the memset problem in MSVC, so the loop can't be optimized to the highest in a lot of cases - preventing this problem from being optimized away. The operators are explicitly not the same, and the languages define this, inherent to the operators, as I said. There is definitely a difference and you should never use the postfix operator unless you intend that side effect. Your example is trivial, in c++ you can add pretty much any operator to any class, it won't be nearly as optimized because it can't. For instance, an iterator won't be optimized like this while a pointer or integer will but an iterator is a much better construct to use. So, if you use the postfix, it will create copies no matter what because that is an already optimized function call. You should always use the prefix, especially in c++.

  • Like 1
Link to comment
Share on other sites
Slice
Slice
Posted
Posted

 

On 5/23/2020 at 11:24 PM, Jief_Machak said:

There is also the solution to create a LoadSVGImage. LoadSVGImage will do what GetBest currently does : the rasterization of ImageSVG and ImageSVGNight. ImageSVG and ImageSVGNight members could be then removed from XIcon.

 

In that later case, no need for XIconSVG and XIconPNG because once loaded, there will be no difference.

The common idea of the changes is to rasterize SVG images immediately before draw. It gives me a possibility to make animation at rasterize level to not keep a set of frames. So I want no cache.

Link to comment
Share on other sites
apianti
apianti
Posted
Posted

You should create a sort of image animator. You only need to rasterize the frame once at any resolution, so you could have a second class that uses a set(?) of images (or something). It could either rasterize as needed and cache or grab from the cache, you can clear it on changes. I haven't exactly looked at how the drawing is happening since the new changes but if everything is based on some drawable object with a virtual function draw() or something it should be pretty easy to implement this behavior over that class in place of the image. You could have a variant for each type of image as well like the images. Which could share a base class as well...

Link to comment
Share on other sites
apianti
apianti
Posted
Posted

Sorry, I wasn't saying you had to keep the cache in the animator, I was saying you could cache the images and you would only need to do that once. For some things this is reasonable, but like what you said, probably not, you would end up with a lot of cached images. Mostly, I was trying to say that you should make a base class that is an animator and then have it perform the function of being an image but it choose what it actually draws at render time based on it's state.

Link to comment
Share on other sites
 Share
Go to topic listing
×
×
  • Create New...