Re: K3 update effects on DB

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(R) 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://altfarm.mediaplex.com/ad/ck/12124-176961-30367-2

__[ 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 ]