----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122678/#review83156 -----------------------------------------------------------
Ship it! I think we should ship this, immediately after the next 2.9 release so there's plenty of testing time. - Boudewijn Rempt On July 29, 2015, 6:20 p.m., Stefano Bonicatti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122678/ > ----------------------------------------------------------- > > (Updated July 29, 2015, 6:20 p.m.) > > > Review request for Calligra. > > > Repository: calligra > > > Description > ------- > > Why is needed: > > The hash of a resource is sometimes calculated on the file, sometimes on the > interpreted (loaded) data which can vary between users or due to > approximations, resulting in the md5 inside the bundle > to be different from the recalculated one. > > > What it does: > > The md5 will be always calculated on the file and not on the interpreted data. > > KoResource::generateMD5 is not pure virtual anymore, because all the > resources has to share the same md5 generation. > > KoResource::saveToDevice is not pure virtual anymore, because we need to set > the md5 to empty each time the resource is saved, so that the next access to > the md5 forces a recalculation. > > KoResource::saveToDevice must be called from the saveToDevice functions of > the inheriting classes. > > saveResourceToStore has been modified so that it doesn't try to save a > resource when a copy cannot be done, when creating a bundle. > Also if the file is writable it should be still copied. > > > What still needs to be done: > > Unfortunately i discovered too late that this fixes only a small portion of > issues, simply because when trying to generate a md5 from a file in a bundle, > that filename or filepath isn't one that points to an existing file on the > disk, but it's a path that has to be interpreted and the bundle has to be > opened. > The idea to fix this is to override the generateMD5 function of all the > bundle resource types and have special code to deal with the path (like > KisPaintOpPreset::load function does), so that a temporary resource store is > created to read the bundle file, otherwise just call > KoResource::generateMD5(). > The problem is that there are resource types as KoColorSet which i'm not > completely sure they should know anything about bundles. > > Even with that fixed there are resource types that cannot go into a bundle, > but where i still removed their generateMD5 implementation, so that the base > KoResource implementation is used. These needs to be tested, to see if i > didn't introduce a regression. > > This commit 8a19e6a469ebe784b038b802c346a80fdbd89b0d conflicts with my patch > and the above reasoning, because it simply prevent bundle files md5 to be > generated. > > I've changed a bit how saveToDevice works, so that it empties the resource > md5 everytime it's saved. This needs to be tested (if the md5, when acquired, > is regenerated) and it should be checked if every possible resource has the > call to KoResource::saveToDevice, otherwise it should be added. > > > Diffs > ----- > > krita/image/brushengine/kis_paintop_preset.h 52caacc > krita/image/brushengine/kis_paintop_preset.cpp 736699d > krita/libbrush/kis_abr_brush.cpp ad717d4 > krita/libbrush/kis_abr_brush_collection.cpp 554865b > krita/libbrush/kis_auto_brush.h c537697 > krita/libbrush/kis_auto_brush.cpp 86cc379 > krita/libbrush/kis_brush.h 1a1bf5c > krita/libbrush/kis_brush.cpp ea389ee > krita/libbrush/kis_gbr_brush.cpp 37158f9 > krita/libbrush/kis_imagepipe_brush.cpp 4d301cb > krita/libbrush/kis_png_brush.cpp 358f8da > krita/libbrush/kis_svg_brush.cpp 2934a36 > krita/plugins/extensions/dockers/tasksetdocker/taskset_resource.h 57ad287 > krita/plugins/extensions/dockers/tasksetdocker/taskset_resource.cpp 6afc96f > krita/plugins/extensions/resourcemanager/resourcebundle.h 7e3c8b9 > krita/plugins/extensions/resourcemanager/resourcebundle.cpp ba535d3 > krita/ui/CMakeLists.txt 195d9b5 > krita/ui/kis_factory2.cc 6e2fd9d > krita/ui/kis_md5_generator.h PRE-CREATION > krita/ui/kis_md5_generator.cpp PRE-CREATION > krita/ui/kis_workspace_resource.h aefc57c > krita/ui/kis_workspace_resource.cpp b71dc49 > libs/pigment/CMakeLists.txt 4f54815 > libs/pigment/resources/KoAbstractGradient.h 167be69 > libs/pigment/resources/KoAbstractGradient.cpp 800a490 > libs/pigment/resources/KoColorSet.h ffd81b8 > libs/pigment/resources/KoColorSet.cpp 47d3c6b > libs/pigment/resources/KoHashGenerator.h PRE-CREATION > libs/pigment/resources/KoHashGeneratorProvider.h PRE-CREATION > libs/pigment/resources/KoHashGeneratorProvider.cpp PRE-CREATION > libs/pigment/resources/KoMD5Generator.h PRE-CREATION > libs/pigment/resources/KoMD5Generator.cpp PRE-CREATION > libs/pigment/resources/KoPattern.h 31acba4 > libs/pigment/resources/KoPattern.cpp 1c1f521 > libs/pigment/resources/KoResource.h 53a18e2 > libs/pigment/resources/KoResource.cpp 2b568bf > libs/pigment/resources/KoSegmentGradient.h bdd6baa > libs/pigment/resources/KoSegmentGradient.cpp 5387743 > libs/pigment/resources/KoStopGradient.h 2e1e3d4 > libs/pigment/resources/KoStopGradient.cpp dc851da > > Diff: https://git.reviewboard.kde.org/r/122678/diff/ > > > Testing > ------- > > Compiles. > The md5 inside the bundle manifests are correct, even for gradients (where > normally the wrong one was used, since they're interpreted). > > > Thanks, > > Stefano Bonicatti > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
