Re: K3 update effects on DB

Took me a few re-reads, but I like it.  Make it so!  My only question is:
what is the name of the graphics operation class in modules that add new
rules?


On Fri, Mar 29, 2013 at 4:36 AM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:

> cc: devel
>
> Hey Bharat,
>
> Yep, it got slept on, but I didn't get a chance to reply yesterday since I
> ended up spending too much time finishing converting all of the modules to
> K3 conventions (woohoo!!!)  :-)
>
> Anyway, a couple thoughts:
> - I'm personally a fan of how the six graphics-related events currently
> work (graphics_rotate, graphics_rotate_completed, graphics_resize,
> graphics_resize_completed, graphics_composite, graphics_composite_completed)
> - I don't think we need to break backward compatibility with the events to
> accomplish what we want
> - As awesome as a module fight would be to see (tonight at 7:30, Foo vs.
> Bar!), I'm not sure it'd be a big deal.  The important thing is that the
> default operation (in the gallery module) goes last.  If a module thinks
> it's super important and wants to avoid all overriding abilities, it can
> just name its operation uniquely and write the graphics rule to match.  In
> the rather odd case that there is still a conflict, the module order module
> would work well.
>
> So, here's my current thought:
> - remove "gallery_graphics::" and/or "GalleryGraphics::" from the graphics
> rules.
> - write a new function in Graphics that runs a generic graphics rule and
> looks something like this:
>
>   static function run_graphics_rule($operation, $input_file, $output_file,
> $options=null, $item=null) {
>     Graphics::init_toolkit();
>
>     $temp_file = System::temp_filename("$operation_",
> pathinfo($output_file, PATHINFO_EXTENSION));
>      Module::event("graphics_$operation", $input_file, $temp_file,
> $options, $item);
>
>     if (@!filesize($temp_file) && method_exists("GraphicsOperations",
> $operation)) {
>       // No events made an image, but we have a standard process to run.
>       if (@filesize($input_file) == 0) {
>          throw new Exception("@todo EMPTY_INPUT_FILE");
>       }
>       call_user_func("GraphicsOperations::$operation", $input_file,
> $temp_file, $options, $item);
>     }
>
>     if (@filesize($temp_file)) {
>       // We generated an output - move it to output_file and run the
> completed event.
>       @rename($temp_file, $output_file);
>       Module::event("graphics_{$operation}_completed", $input_file,
> $output_file, $options, $item);
>     }
>   }
>
> - move the guts of the three GalleryGraphics functions to
> GraphicsOperations.  Note that these three functions all currently follow
> the same pattern as above with regard to init toolkit, calling events,
> checking input/output sizes, etc., so they can be largely trimmed.
> - add GraphicsOperations::crop().  This isn't there currently, but
> probably should be and it's easy to do.
>
> Here's some examples of how existing modules/themes would work with it:
> - themes - change the "resize" graphics rule to match what's desired.
> - watermark - add a "composite" graphics rule
> - square_thumbs - add a "crop" graphics rule
> - jpegtran - add a "graphics_rotate" event
> - image_optimizer - add an "optimize" graphics rule, then add a
> "graphics_optimize" event
> - quotas - add a "graphics_rotate" event (doesn't actually do anything
> with the image)
> - keeporiginal - add a "graphics_rotate" event (doesn't actually do
> anything with the image)
>
> Advantages:
> - largely/entirely backwards compatible
> - gives us the flexibility we (or at least I) want
> - makes the DB entries for graphics rules cleaner
> - eliminates my confusion between Graphics and GalleryGraphics
>
> As a separate item, I like your idea of bringing the graphics rules into
> the UI, but not sure how to do it well.  Perhaps a decent first approach is
> to make it part of the advanced settings screen, then see if users demand
> something prettier?
>
> Your thoughts?
>
> Take care,
> Shad
>
>
> On 29 March 2013 05:26, Bharat Mediratta <[email protected]> wrote:
>
>>
>> On Wed, Mar 27, 2013 at 4:14 PM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:
>>
>>> Hey Bharat,
>>>
>>> Hmm... an interesting idea!  That said, I'm not yet convinced we need
>>> the extra flexibility.  While I think the idea of letting modules both add
>>> their own operations and replace the existing ones is good, I think we
>>> already get it with graphics rules and events and wonder if making it a
>>> hook might be overkill.  In particular, I think that:
>>> - the 6 events that GalleryGraphics calls already allow a module to
>>> replace or supplement rotate/resize/composite
>>> - implementing a structure with the three fixed names doesn't allow us
>>> to do other things (of course, this can easily be made flexible still
>>> sticking with your basic concept).
>>>
>>> I can think of four examples to use as cases to illustrate what I mean...
>>> - gallery module: the basic implementation.  It adds one graphics rule
>>> to resize for thumb/resize generation.
>>> - watermark module: add a new graphics rule to run composite after the
>>> resize during thumb/resize generation.
>>> - jpegtran module: can use a graphics_rotate event to use jpegtran to
>>> losslessly rotate jpg images.
>>> - image_optimizer module: adds its own "optimize" operation as the last
>>> step in thumb/resize generation. (*)
>>>
>>
>> I've thought about this some more and it does make sense to have directly
>> addressable graphics operations.  For example gallery_graphics::resize does
>> something very specific.  If we made a new foo_graphics::resize which was
>> similar but different, the expectation would be that when you enable the
>> foo module it updates the graphics rules to use its version of resize.  If
>> you have a bar module with its own resize, which one do you use?  Do they
>> fight?  Module order would be the arbiter and that's a little weird.
>>
>> I think it makes more sense to have modules provide graphics rules, and
>> then let the user decide which rule they want to use.  We'd need a UI, but
>> I think the end result would be more valuable for the user.
>>
>> But that approach is very similar to tasks - each module can provide a
>> set of tasks in its own hook.  We call FooTask::available_tasks() to get
>> the list, then interact with each task directly.  This still falls in the
>> realm of the Hook approach so I'm ok with it.  Note that in this approach,
>> one graphics rule cannot replace another one.
>>
>>
>>>
>>> To address your three specific points:
>>> 1)  Yes, we definitely do need to make a unified hook interface!  It's
>>> on my very short list once I get done translating all the modules to K3.
>>> 2)  I *do* like that this gets the name "GalleryGraphics" out of the DB.
>>> 3)  Yes - prrrrrofit!  (http://www.youtube.com/watch?v=tO5sxLapAts)
>>>
>>> Hmm... maybe I need to sleep on this and rethink in the morning.
>>>
>>
>> How'd the overnight though process go? :-)
>>
>> -Bharat
>>
>
>

------------------------------------------------------------------------------
Own the Future-Intel&reg; Level Up Game Demo Contest 2013
Rise to greatness in Intel's independent game demo contest.
Compete for recognition, cash, and the chance to get your game 
on Steam. $5K grand prize plus 10 genre and skill prizes. 
Submit your demo by 6/6/13. http://p.sf.net/sfu/intel_levelupd2d

__[ g a l l e r y - d e v e l ]_________________________

[ list info/archive --> http://gallery.sf.net/lists.php ]
[ gallery info/FAQ/download --> http://gallery.sf.net ]