Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add image handling for waveColor and progressColor #2348

Closed
wants to merge 21 commits into from

Conversation

califken
Copy link
Contributor

Short description of changes:

Added image handling, in addition to the previous commits that added gradient handling, to the waveColor and waveProgress parameters.

wavesurfer-js-Image-Fill-Styles-Example

Breaking in the external API:

None

Breaking changes in the internal API:

None

Todos/Notes:

Related Issues and other PRs:

@coveralls
Copy link

coveralls commented Aug 28, 2021

Coverage Status

Coverage decreased (-0.1%) to 81.616% when pulling 41b74a8 on califken:image-fill-styles into be6e492 on katspaugh:master.

src/drawer.canvasentry.js Outdated Show resolved Hide resolved
src/drawer.canvasentry.js Outdated Show resolved Hide resolved
@sundayz
Copy link
Contributor

sundayz commented Aug 28, 2021

Nice! Those are some cool visual effects.

@thijstriemstra
Copy link
Contributor

really cool! ill review more later

CHANGES.md Outdated Show resolved Hide resolved
example/image-fill-styles/main.js Outdated Show resolved Hide resolved
src/drawer.canvasentry.js Outdated Show resolved Hide resolved
src/drawer.canvasentry.js Outdated Show resolved Hide resolved
@thijstriemstra thijstriemstra changed the title enhancement: add image handling for waveColor and progressColor param… add image handling for waveColor and progressColor Aug 31, 2021
update 2 images in image fill style example page
update: add image attribution list to example
fix: fixed indentation in example
src/wavesurfer.js Outdated Show resolved Hide resolved
src/wavesurfer.js Outdated Show resolved Hide resolved
src/wavesurfer.js Outdated Show resolved Hide resolved
src/wavesurfer.js Show resolved Hide resolved
src/wavesurfer.js Show resolved Hide resolved
Copy link
Contributor Author

@califken califken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

example/image-fill-styles/index.html Outdated Show resolved Hide resolved
src/wavesurfer.js Show resolved Hide resolved
src/wavesurfer.js Show resolved Hide resolved
src/drawer.canvasentry.js Outdated Show resolved Hide resolved
src/drawer.canvasentry.js Outdated Show resolved Hide resolved
example/image-fill-styles/index.html Outdated Show resolved Hide resolved
example/image-fill-styles/main.js Outdated Show resolved Hide resolved
example/image-fill-styles/main.js Outdated Show resolved Hide resolved
spec/wavesurfer.spec.js Outdated Show resolved Hide resolved
fix: add strict equality comparison to object type check in get fill style
fix: change let to var in examples
fix: change var to let or const in tests
CHANGES.md Outdated Show resolved Hide resolved

// if it is an object, handle it as a CanvasImageSource
if (typeof color === "object") {
return ctx.createPattern(color, "repeat");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now that we have new methods, let's think about allowing user to change "repeat" value (and other settings in new method). Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add an optional options parameter to setWaveColor, setWaveStyle, etc.. that could include options depending on the type of style being set. If it's a gradient, options for controlling the angle of the gradient. If it's a Canvas Image Source, the repeat value.

setWaveStyle(myCanvImgSrc, {repeat: 'repeat-x'});

Are you looking all the way back up to the WaveSurfer parameters object? Or just up to the new methods?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you looking all the way back up to the WaveSurfer parameters object? Or just up to the new methods?

Yep, both.

Copy link
Contributor Author

@califken califken Sep 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My recommendation is a new Wavesurfer Params object property called waveFillStyle, which would be an object type that could also be plugged into progressFillStyle, and eventually the wave background, the playhead, etc..)

  generateWaveform() {
    ...,
    waveFillStyle: {
      
      // only one of these (color, gradient, image, canvas, video) would be required
      color: {'red'},

      // each type of fill style would have its options as siblings of its data
      gradient: {
          colorstops: [
              [0.0,'red'],
              [0.5,'blue'],
              [1.0,'purple']
          ],
          coordinates: [0,0,500,500]
      },
      image: {
          src: document.querySelector("#image-element"),
          repeat: 'repeat'
      },

      // not yet, but eventually could pass any canvas as the style
      canvas: {
          element: document.querySelector("#canvas-element")
      },

      // I was able to pass in a canvas video as the background, it is pretty great,
      // but was quite a bit of setup, as in having to run an interval to update the canvas
      // from the video element, so still sorting the last pieces of that out before I 
      // push anything
      video: {
          element: document.querySelector("#video-canvas")
      }

  }

And keeping waveColor, etc.. as is for backwards compatibility.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's start with setWaveStyle(myCanvImgSrc, {repeat: 'repeat-x'}); like you suggested. The object passed to setWaveStyle can be called options. We can later decide what this object is called in the main wavesurfer options.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In setWaveStyle, the style is being set to the waveColor parameter, and then drawBuffer is called. How do I get access to this options object over in drawer.canvasentry.js's getFillStyle method?

From what I can tell, I will need to

  • set the options object as a params property
  • add that property as an argument to drawer.multicanvas.js's call to setFillStyles line 556
  • add the wave and progressStyleOptions parameter to drawer.canvasentry.js's setFillStyles method line 156
  • pass the new arguments to the getFillStyle calls lines 158 and 161
  • subsequently pass it getFillStyle's ctx.createPattern call, where the repeat option can be utilized.

Is this what you had in mind? It is what I am committing right now, and it works.

If this is not on track with what you were looking for, I will need more details on how else to make this happen. Let me know! Thanks

spec/wavesurfer.spec.js Outdated Show resolved Hide resolved
@califken
Copy link
Contributor Author

I've added 2 more examples in the examples directory, a canvas fill style and a video fill style example. I wanted to share where these are at. Not sure if we actually should move any of the logic further into WaveSurfer, when it can reside in the client code just fine, as it does in the examples. Let me know your thoughts.

A couple points -

  • The canvas effect is from a pen on Codepen, and is attributed on the page.
  • On the video page, the video and audio are mine. I thought in this case, it might be warranted to use something beyond the demo to demonstrate how this might be utilized. Should I keep them there? Or use the demo.mp3 and find something suitable to display with that? Let me know. Whichever way we go , it's all good.

example/video-fill-style/index.html Show resolved Hide resolved
example/video-fill-style/main.js Outdated Show resolved Hide resolved
*/
setFillStyles(waveColor, progressColor) {
this.waveCtx.fillStyle = this.getFillStyle(this.waveCtx, waveColor);
setFillStyles(waveColor, progressColor, waveStyleOptions, progressStyleOptions) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about using setFillStyles(waveStyleOptions, progressStyleOptions) and extract the colors from the options. setWaveColor etc would have to be adjusted as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the waveColor parameter would accept an object containing: the "style" (color, gradient, etc..) as a property, and the options for that style as a property?

update canvas and video scripts and code examples
example/canvas-fill-style/index.html Outdated Show resolved Hide resolved
example/canvas-fill-style/main.js Outdated Show resolved Hide resolved
@thijstriemstra
Copy link
Contributor

Should I keep them there? Or use the demo.mp3 and find something suitable to display with that? Let me know. Whichever way we go , it's all good.

Let's keep the amount of demo 'support files' to a minimum.

@thijstriemstra
Copy link
Contributor

any update on this @califken so we can look at releasing 6.0.0?

@katspaugh katspaugh closed this Feb 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants