Is Scald supports picture group provided by picture module. As we can define for each context the image styles to use, can we use a "style" picture group ?

CommentFileSizeAuthor
#7 scald-1935570-picture-7.patch4.12 KBjcisio
#5 add-picture-support-1935570-5.patch2.08 KBnagy.balint

Comments

jcisio’s picture

jcisio commented

Not yet, but I think it's a good idea. Scald Image can be easily extended to support Picture module.

flocondetoile’s picture

flocondetoile
Primary language French
Location Lyon
commented

I tried to modify in scald_image.module the function which load image style line 49

/**
 * Implements hook_scald_transcoders().
 */
function scald_image_scald_transcoders() {
  $transcoders = array();
  foreach (image_styles() as $name => $style) {
    $transcoders['style-' . $name] = array(
      'title' => t('@style (Image style)', array('@style' => $name)),
      'description' => t('Use the Image style @style to prepare the image', array('@style' => $name)),
      'formats' => array(
        'image' => 'passthrough'
      )
    );
  }
   
  return $transcoders;
}

I simply add in this function an another foreach to load picture group

I obtain this

/**
 * Implements hook_scald_transcoders().
 */
function scald_image_scald_transcoders() {
  $transcoders = array();
  foreach (image_styles() as $name => $style) {
    $transcoders['style-' . $name] = array(
      'title' => t('@style (Image style)', array('@style' => $name)),
      'description' => t('Use the Image style @style to prepare the image', array('@style' => $name)),
      'formats' => array(
        'image' => 'passthrough'
      )
    );
  }
  if (module_exists('picture') {
    foreach (picture_mapping_load_all() as $name => $style) {
      $transcoders['style-' . $name] = array(
        'title' => t('@style (Image style)', array('@style' => $name)),
        'description' => t('Use the Image style @style to prepare the image', array('@style' => $name)),
        'formats' => array(
          'image' => 'passthrough'
        )
      );
    }
  }  
  return $transcoders;
}

Seems to work fine. the picture group are well present in the select list transcoder.

But the image rendered is then broken. There is yet some work to do.

jcisio’s picture

jcisio commented

Picture groups are not image styles, so I think we need a different prefix (like "group-") for Picture groups. Also, yes, render function needs to be changed too.

jcisio’s picture

jcisio commented
Title: support picture module » Support Picture module
Version: 7.x-1.0-rc1 » 7.x-1.x-dev
Issue tags: +Scald-1.1
nagy.balint’s picture

nagy.balint commented
Category: feature » task
Status: Active » Needs review
StatusFileSize
new add-picture-support-1935570-5.patch2.08 KB

Okey here is my first try.

I incorporated the code snippet by flocondetoile with the fix you mentioned.

Since the purpose of the module is to be able to define image styles for certain breakpoints, I only implemented mode=atom and in all other cases the function just returns. So I did not populate the transcoded properties on the atom object.

I determine the fallback image style the same way as the picture module does it when it is set to Automatic.
(Basically just taking the first image style we find)

jcisio’s picture

jcisio commented

Nice try. Really.

I was thinking of using properly the transcoder: the transcoder return a structured array of transcoded urls, with their properties (multiplier/breakpoint), then the player will do the rest. Advantage is that we can use the HTML5 figure/figcaption player (that encapsulates the 'picture', with a nice caption), but then it would be more complex: image players have to take care of multiple transcoded urls (easy part, because they can simply take the fallback url if they do not support RWD), and have to clone code from Picture module (subtle part!). I haven't look at that part of Picture to see if there are many lines of code.

Keep the status as NR because I think I'll be back when I have more time investigate the Picture module.

jcisio’s picture

jcisio commented
StatusFileSize
new scald-1935570-picture-7.patch4.12 KB

Here is a recent patch. The same principle, but I arranged code a little and let the HTML5 player support Picture, too.

jcisio’s picture

jcisio commented
Status: Needs review » Needs work

I'll reroll the patch.

jcisio’s picture

jcisio commented
Status: Needs work » Fixed

Ok, I rerolled it, tested a bit and decided to commit! Thanks all.

Automatically closed -- issue fixed for 2 weeks with no activity.