EInvalidOp in MakeAlphaNonZeroUP

[zedxxx]

EInvalidOp in MakeAlphaNonZeroUP

[andersmelander]

Ah... Yes of course; The cache is global and there's no lock on it.

This means that VPR_CACHE should not be defined by default. It should only be defined when one is sure that only one thread, at a time, will use VPR.

I will probably end up completely removing all the VPR_CACHE stuff since it didn't really provide the performance improvements I had hoped for.

[andersmelander]

I'll probably need a reproducible test case if I'm to anything about this. As you can see from the try...except you are not the first to get an overflow there.

[zedxxx]

This try-except block is mine.

[andersmelander]

Ah :-) but still; Not the first.

[zedxxx]

Undefining VPR_CACHE in GR32_VPR.pas fixes the issue. Is there something wrong with RenderPolyPolygon?

[andersmelander]

@zedxxx Did you delete a comment?

Is everything right here? https://github.com/graphics32/graphics32/blob/master/Source/GR32_VPR.pas#L579

Yes, as far as I can see it's okay - but I should have commented the code. It increases the allocated size in blocks of GrowSpanDataCacheSize.

graphics32/Source/GR32_VPR.pas

Lines 574 to 582 in 13c296e

	if (Size > SpanDataCacheSize) then
	begin
	if (Size < MinSpanDataCacheSize) then
	SpanDataCacheSize := MinSpanDataCacheSize
	else
	SpanDataCacheSize := (Size + GrowSpanDataCacheSize - 1) and (not (GrowSpanDataCacheSize-1));
	GetMem(SpanDataCache, SpanDataCacheSize);
	end;

[zedxxx]

Yes, I realized that this is not the case.

BTW, GetMem on line 581 needs to be replaced with ReallocMem. Right?

[andersmelander]

BTW, GetMem on line 581 needs to be replaced with ReallocMem. Right?

Yes. You are right.

[zedxxx]

Oh, its multi-threading problem (application use background threads to render bitmaps).

After replacing var with threadvar for global variables all works fine, but I'm not sure about the memory leaks.
Perhaps using TCriticalSection is the best solution here.

[andersmelander]

Perhaps using TCriticalSection is the best solution here.

No because it's not only the allocation and reallocation that needs to be protected. It's also the content of the buffer. Only one rasterizer at a time can use the buffer, and locking the buffer while one thread is using it would effectively serialize rasterization.