Hi, I just had a closer look, and summary of it all: I like!
I have found one thing that differs from before... not sure if this is something you care about or not, but... before your changes, the code would adapt the aspect ratio to the original picture format. For example, if I chose 5:4 and the picture is vertical (portrait), the aspect ratio would automagically be auto-flipped to 4:5. This isn't happening in your branch. It's really a small thing, I'm not even sure if I'd be annoyed... Also, while speaking of flipping the aspect ratio, I've always wondered if the aspect flipping shouldn't be saved... from looking at your code, all the needed changes would be in aspect_presets_changed, all that's needed is to care about the sign of d and p->ratio_d, wouldn't it? Last comment, from the pure code management point of view; I noticed that the aspect ratio data exists in three places in the code. My gut feeling says that this can be a source of errors (inconsistent updates of numbers). I'd suggest having those numbers in a constant array, ratios[RATIO_COUNT][2] or something like that and use those... The way I see it, this does replace PR 159. I'll simply delete the corresponding branch. Cheers, Richard In message <512d2421.9090...@gmail.com> on Tue, 26 Feb 2013 22:07:45 +0100, AlicVB <alic...@gmail.com> said: alic.vb> Hi all, alic.vb> I finally let the mask in paused for 1 or 2 days and work on correcting alic.vb> bug in C&R. alic.vb> I've created a new branch on main repo for that : CR_fixes alic.vb> Currently here's what has been done : alic.vb> 1- save changes done with cropping area even if the pipe is still alic.vb> working (bug #9245) alic.vb> 2- save ratio in image params (should close lot of bugs/feature request alic.vb> like #9151 or #9143 and replace somehow PR #159) alic.vb> 3- avoid cropping area to "jump" when C&R module get focus alic.vb> 4- exported image now respect exactly the ratio (partially fix #9148) alic.vb> 5- different little other fixes alic.vb> alic.vb> The main "tricky" point is 4 : alic.vb> this indeed correct a "bug", but the weird effect is that it will change alic.vb> the exported size of all already processing pictures. The change is not alic.vb> really big (max 6 pixels) but... alic.vb> And I don't think it's possible to handle that in legacy_params, because alic.vb> we would need to access the whole pipe, or at least all other distort alic.vb> modules to do that (the size change is dependent of the size of the alic.vb> image just before C&R is applied, so after all previous iop have been alic.vb> applied) alic.vb> alic.vb> If some of you have other bugs to fix - in C&r, I mean ;) - please try alic.vb> the branch to see if it's still present and report it. If you are lucky, alic.vb> I'll be able to correct it ! alic.vb> alic.vb> Oh, and as usual : backup _everything_ before testing. I've upgraded C&R alic.vb> params. The upgrade should be handle properly, but not the downgrade ;) alic.vb> alic.vb> Thanks for your comments and tests. alic.vb> Aldric alic.vb> alic.vb> ------------------------------------------------------------------------------ alic.vb> Everyone hates slow websites. So do we. alic.vb> Make your web apps faster with AppDynamics alic.vb> Download AppDynamics Lite for free today: alic.vb> http://p.sf.net/sfu/appdyn_d2d_feb alic.vb> _______________________________________________ alic.vb> darktable-devel mailing list alic.vb> darktable-devel@lists.sourceforge.net alic.vb> https://lists.sourceforge.net/lists/listinfo/darktable-devel ------------------------------------------------------------------------------ Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_d2d_feb _______________________________________________ darktable-devel mailing list darktable-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/darktable-devel
