Tile data management changed in Krita

[Dmitry Kazakov]

Hi, Fazek!

Your patch is really dangerous, especially after we announced the alpha
version. The thread-safe access to the tile data object is guaranteed by
the wrapper object KisTile. It puts ref() and deref() calls every tile the
tile data is accessed. The same applies to the swapper and pooler threads.
If you see a problem in thread safety, please describe it here or write a
test case. We should also check how the performance is affected by this
change. I think the best option now is ti revert this patch and put to the
review board with th explanation of the problem it fixes.

On Mon, Mar 21, 2016 at 2:50 PM, Boudewijn Rempt <boud at valdyas.org> wrote:

> Hi Fazek,
>
> This is quite dangerous... It's typically the sort of commit that should
> go through review and testing first because it's the core-most part of
> Krita. I also get a build failure now, so I'm guessing you're not building
> the unittests?
>
> Scanning dependencies of target KisLowMemoryTests
> [ 50%] Building CXX object
> libs/image/tiles3/tests/CMakeFiles/KisLowMemoryTests.dir/kis_low_memory_tests.cpp.o
> /home/boud/dev/krita/libs/image/tiles3/tests/kis_low_memory_tests.cpp: In
> member function ‘void KisLowMemoryTests::hangingTilesTest()’:
> /home/boud/dev/krita/libs/image/tiles3/tests/kis_low_memory_tests.cpp:167:52:
> error: cannot convert ‘KisTileDataSP {aka QSharedPointer<KisTileData>}’ to
> ‘KisTileData*’ in initialization
>      KisTileData *weirdTileData = dstTile->tileData();
>                                                     ^
> /home/boud/dev/krita/libs/image/tiles3/tests/kis_low_memory_tests.cpp:180:52:
> error: cannot convert ‘KisTileDataSP {aka QSharedPointer<KisTileData>}’ to
> ‘KisTileData*’ in initialization
>      KisTileData *cowedTileData = dstTile->tileData();
>                                                     ^
> [ 50%] Linking CXX executable KisPainterTest
> libs/image/tiles3/tests/CMakeFiles/KisLowMemoryTests.dir/build.make:62:
> recipe for target
> 'libs/image/tiles3/tests/CMakeFiles/KisLowMemoryTests.dir/kis_low_memory_tests.cpp.o'
> failed
> make[2]: ***
> [libs/image/tiles3/tests/CMakeFiles/KisLowMemoryTests.dir/kis_low_memory_tests.cpp.o]
> Error 1
> CMakeFiles/Makefile2:17296: recipe for target
> 'libs/image/tiles3/tests/CMakeFiles/KisLowMemoryTests.dir/all' failed
> make[1]: ***
> [libs/image/tiles3/tests/CMakeFiles/KisLowMemoryTests.dir/all] Error 2
>
>
>
> On Mon, 21 Mar 2016, Fazekas László wrote:
>
> Hello all,
>>
>> I just modified the internal logic of the tile data management in Krita
>> (T1835 in phabricator). If you encounter any strange behaviour related to
>> tiles please tell me. Specially if the program crashes or complains about
>> remaining tiles when exiting.
>>
>> Thanks,
>> Fazek
>>
>> _______________________________________________
>> Krita mailing list
>> kimageshop at kde.org
>> https://mail.kde.org/mailman/listinfo/kimageshop
>>
>>
> --
> Boudewijn Rempt | http://www.krita.org, http://www.valdyas.org
> _______________________________________________
> Krita mailing list
> kimageshop at kde.org
> https://mail.kde.org/mailman/listinfo/kimageshop
>
>


-- 
Dmitry Kazakov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kimageshop/attachments/20160321/8ecf3ec3/attachment.html>