----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122678/#review76521 -----------------------------------------------------------
If my patch to make reloading resources from a bundle conflicts, feel free to change it away -- it's just that for presets (and I think only for presets), we should be able to restore to pristine state at runtime. Maybe we should just keep the full byte array of a resource in memory... - Boudewijn Rempt On Feb. 22, 2015, 5:14 p.m., Stefano Bonicatti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122678/ > ----------------------------------------------------------- > > (Updated Feb. 22, 2015, 5:14 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/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 eefaef8 > krita/libbrush/kis_brush.h be50b5d > krita/libbrush/kis_brush.cpp ccf3074 > krita/libbrush/kis_gbr_brush.cpp 5e5de22 > libs/pigment/resources/KoResource.h e5ac7aa > libs/pigment/resources/KoResource.cpp d64c460 > libs/pigment/resources/KoSegmentGradient.h 42cded3 > libs/pigment/resources/KoSegmentGradient.cpp 48bf563 > libs/pigment/resources/KoStopGradient.h ea98ed2 > libs/pigment/resources/KoStopGradient.cpp 83ad3bf > krita/image/brushengine/kis_paintop_preset.cpp e344b1f > krita/image/brushengine/kis_paintop_preset.h 58b4347 > krita/libbrush/kis_imagepipe_brush.cpp c49f74a > krita/libbrush/kis_png_brush.cpp 4cde8f1 > 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 d161e75 > krita/ui/kis_workspace_resource.h aefc57c > krita/ui/kis_workspace_resource.cpp b71dc49 > libs/pigment/resources/KoAbstractGradient.h d6d6d4f > libs/pigment/resources/KoAbstractGradient.cpp 0f0fa5f > libs/pigment/resources/KoColorSet.h 810bda33 > libs/pigment/resources/KoColorSet.cpp a8f8821 > libs/pigment/resources/KoPattern.h 14bc3da > libs/pigment/resources/KoPattern.cpp 4c72646 > > 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
