Change Reference Example #7280

samarsrivastav · 2024-09-22T12:58:27Z

Resolves #7228

Changes:

Screenshots of the change:

npm run lint passes
Inline documentation is included / updated
Unit tests are included / updated

welcome · 2024-09-22T12:58:29Z

🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page!

Qianqianye · 2024-09-25T18:47:23Z

Thanks @samarsrivastav for working on it. Tagging @aleannab and @davepagurek to review this PR.

perminder-17

Thanks @samarsrivastav for your work, it's a great work and thanks @aleannab for rasing and explaing the issue with solution. What I feel the sketch @aleannab mentioned has only used two colors for all the types of blendMode. They are #1a85ff and #d41159.

However in the examples if I do grunt yui:dev I can see the noticable difference with the examples.

What I see that you have used the set of colors from here: (in the refrence) which is actually the resultant colors. :")

We can fix this by using the appropriate colors ('#1a85ff' and '#d41159') for them, and can add other blendModes which are removed i.e. "ADD" and "DARKEST" (with different color sets). Otherwise looks good. :)

perminder-17 · 2024-09-26T00:23:59Z

src/core/rendering.js

 *
 *   // Set the blend mode.
- *   blendMode(ADD);
+ *   blendMode(HARD_LIGHT);


I noticed that you removed two example sketches:

blendMode(ADD)

blendMode(DARKEST)

I think we could keep these examples. What do you think?

Hey @perminder-17 !! I have added these two as well, is there any sort of order we need to follow while adding all the blendmodes?

Hey @perminder-17 !! I have added these two as well, is there any sort of order we need to follow while adding all the blendmodes?

No, i don't think so. Its just that we need to help users learn about blendMode() so it could be in any order.

aleannab · 2024-09-26T01:22:27Z

src/core/rendering.js

@@ -728,7 +728,7 @@ p5.prototype.clearDepth = function(depth) {
 * final color results from blending the source pixel's color with the canvas
 * pixel's color. RGB color values from the source and canvas pixels are
 * compared, added, subtracted, multiplied, and divided to create different
- * effects. Red values with red values, greens with greens, and blues with
+ * effects. #d41159 values with #d41159 values, greens with greens, and blues with


Shouldn't this stay red, as this statement refers to the color channels, not the color of the line?

Good catch!

aleannab · 2024-09-26T01:25:30Z

src/core/rendering.js

 *   line(75, 25, 25, 75);
 *
- *   describe('A blue line and a red line form an X on a gray background.');
+ *   describe('A #1a85ff line and a #d41159 line form an X on a gray background.');


In order to be more screen reader-accessible, I suggest we are more descriptive with the new color values instead of using hex codes like #d41159. For example, here we could say "A vibrant pink line and a bright blue line..."

This comment would be applicable to all the describe statements for the blend mode examples.

Hey @aleannab, I completely understand the need to use color names, but I’m a bit uncertain about the best way to name these specific colors. Could you provide some guidance on how to approach this? I’d really appreciate your help!

aleannab · 2024-09-26T01:36:47Z

What I see that you have used the set of colors from here: (in the refrence)

Ah, yes, in the original reference I used '#1a85ff' and '#d41159' for all the examples.

The colors in the table are the resulting colors after the blend mode has been applied. I provided those to highlight the differences in the before and after.

Here's example of how I generated the reference images:
https://editor.p5js.org/aleannab/sketches/r4VWTS7Cr

This is how the end results should show, as I mentioned in the original issue.

perminder-17 · 2024-09-26T01:46:21Z

What I see that you have used the set of colors from here: (in the refrence)

Ah, yes, in the original reference I used '#1a85ff' and '#d41159' for all the examples.

The colors in the table are the resulting colors after the blend mode has been applied. I provided those to highlight the differences in the before and after.

Here's example of how I generated the reference images: https://editor.p5js.org/aleannab/sketches/r4VWTS7Cr

Aaah...yess. I see, I think currently we are using the resulting colors rather than the colors you mentioned in the refrence i.e. '#1a85ff' and '#d41159'. We should use these colors only rather than resulting one. I just now noticed in the issue haha. Thanks @aleannab I'll now edit my review so that @samarsrivastav could work on it. Thanks:)

aleannab · 2024-09-26T02:06:39Z

What I see that you have used the set of colors from here: (in the refrence)

Ah, yes, in the original reference I used '#1a85ff' and '#d41159' for all the examples.
The colors in the table are the resulting colors after the blend mode has been applied. I provided those to highlight the differences in the before and after.
Here's example of how I generated the reference images: https://editor.p5js.org/aleannab/sketches/r4VWTS7Cr

Aaah...yess. I see, I think currently we are using the resulting colors rather than the colors you mentioned in the refrence i.e. '#1a85ff' and '#d41159'. We should use these colors only rather than resulting one. I just now noticed in the issue haha. Thanks @aleannab I'll now edit my review so that @samarsrivastav could work on it. Thanks:)

Kind of along the same note: I think we should change the REPLACE, REMOVE, and SUBTRACT examples to use these colors as well. The only reason why I didn't include it in my original reference was because I could only use these in WEBGL mode (and others only in 2D mode).

perminder-17 · 2024-09-26T02:20:29Z

Kind of along the same note: I think we should change the REPLACE, REMOVE, and SUBTRACT examples to use these colors as well. The only reason why I didn't include it in my original reference was because I could only use these in WEBGL mode (and others only in 2D mode).

Yes, sure thing. I checked these three modes too, and the colors you mentioned work great.

samarsrivastav · 2024-09-26T13:53:49Z

Hey @perminder-17 and @aleannab, I've resolved the color issue and added the colors as per the example provided here. However, I'm still uncertain about how to best name these colors. I'd appreciate any guidance you can offer on the naming part!

samarsrivastav · 2024-09-26T14:16:58Z

I’ve used an AI assistant to generate color names for my reference. Could you please take a look and let me know your thoughts?

 blendModes = [
  {
    blendMode: 'BLEND',
    description: 'A dodgerblue line and a cerise line form an X on a gray background.'
  },
  {
    blendMode: 'HARD_LIGHT',
    description: 'A faint dodgerblue line and a faint cerise line form an X on a gray background. The area where they overlap is purple.'
  },
  {
    blendMode: 'ADD',
    description: 'A faint dodgerblue line and a faint cerise line form an X on a gray background. The area where they overlap is purple.'
  },
  {
    blendMode: 'DARKEST',
    description: 'A faint dodgerblue line and a faint cerise line form an X on a gray background. The area where they overlap is purple.'
  },
  {
    blendMode: 'BURN',
    description: 'A dodgerblue line and a cerise line form an X on a gray background. The area where they overlap is black.'
  },
  {
    blendMode: 'LIGHTEST',
    description: 'A faint dodgerblue line and a faint cerise line form an X on a gray background. The area where they overlap is lavender.'
  },
  {
    blendMode: 'EXCLUSION',
    description: 'A dodgerblue line and a cerise line form an X on a gray background. The area where they overlap is greenish-gray.'
  },
  {
    blendMode: 'MULTIPLY',
    description: 'A dodgerblue line and a cerise line form an X on a gray background. The area where they overlap is very dark blue.'
  },
  {
    blendMode: 'SCREEN',
    description: 'A faint dodgerblue line and a faint cerise line form an X on a gray background. The area where they overlap is very light lavender.'
  },
  {
    blendMode: 'REPLACE',
    description: 'A diagonal cerise line.'
  },
  {
    blendMode: 'REMOVE',
    description: 'The silhouette of an X is missing from a gray background.'
  },
  {
    blendMode: 'DIFFERENCE',
    description: 'A dodgerblue line and a cerise line form an X on a gray background. The area where they overlap is dark brown.'
  },
  {
    blendMode: 'OVERLAY',
    description: 'A faint dodgerblue line and a faint cerise line form an X on a gray background. The area where they overlap is medium purple.'
  },
  {
    blendMode: 'SOFT_LIGHT',
    description: 'A faint dodgerblue line and a faint cerise line form an X on a gray background. The area where they overlap is lavender gray.'
  },
  {
    blendMode: 'DODGE',
    description: 'A faint dodgerblue line and a faint cerise line form an X on a gray background. The area where they overlap is pale lavender.'
  },
  {
    blendMode: 'SUBTRACT',
    description: 'A dodgerblue line and a cerise line form an X on a gray background. The area where they overlap is black.'
  }
];

perminder-17 · 2024-09-26T16:41:59Z

Hey @perminder-17 and @aleannab, I've resolved the color issue and added the colors as per the example provided here.

hey, thanks. Looks correct.

I'd appreciate any guidance you can offer on the naming part!

Yes, in my opinion, if you're referring to the color dodgerblue, its color code is #1E90FF, which is very similar to the color you used in the sketch (i.e. #1a85ff). However, these are not the exact same colors, and the color you used in the sketch doesn't have an official name currently :"(

We could just use the color code in the discription saying for eg.

  {
    blendMode: 'BLEND',
    description: 'A `#1a85ff` line and a `#d41159` line form an X on a gray background.'
  },

similarly for other colors, we can say,

{
    blendMode: 'DODGE',
    description: 'A `#c8ffff` (from the image shared below) and a `ffc1ff` line form an X on a gray background. 
                 The area where they overlap is (the results after calculations).' // need to calculate the overlap color
  },

To find the exact colors (resultant) that we get after blending, here are the final colors produced.

The reasons I'm not focusing on the color names are:

Some types look exactly the same—for example, "ADD" and "DODGE"—and we can distinguish them by their color codes.
Including names that have different color codes in the description could confuse users.

For calculating the exact colors overlapped, you can calculate from the formulas
https://github.com/pdaoust/stylus-helpers/blob/master/blend.styl
or
https://en.wikipedia.org/wiki/Blend_modes

Or you may use ai for the calculations, since it's probably very good at calculations haha. Let me know what are your thoughts.

perminder-17 · 2024-09-26T16:47:05Z

One last thought: as @aleannab mentioned, can we use the same two colors, (1a85ff and d41159), in the blendMode(SUBTRACT)? I believe you're currently using blue and red, correct?

aleannab · 2024-09-26T17:05:03Z

We could just use the color code in the discription saying for eg.

  {
    blendMode: 'BLEND',
    description: 'A `#1a85ff` line and a `#d41159` line form an X on a gray background.'
  },

similarly for other colors, we can say,

{
    blendMode: 'DODGE',
    description: 'A `#c8ffff` (from the image shared below) and a `ffc1ff` line form an X on a gray background. 
                 The area where they overlap is (the results after calculations).' // need to calculate the overlap color
  },


The reasons I'm not focusing on the color names are:

    1. Some types look exactly the same—for example, "ADD" and "DODGE"—and we can distinguish them by their color codes.

    2. Including names that have different color codes in the description could confuse users.


For calculating the exact colors overlapped, you can calculate from the formulas https://github.com/pdaoust/stylus-helpers/blob/master/blend.styl or https://en.wikipedia.org/wiki/Blend_modes

Or you may use ai for the calculations, since it's probably very good at calculations haha. Let me know what are your thoughts.

I also agree that the exact color names aren't super important. However, considering that the describe function is for screen reader accessibility for blind or visually impaired users to read the text, I think that having semi-descriptive color names (e.g. pale blue vs. aqua) are more useful than hex codes. It does get tricky though since many of the colors do look similar.

Maybe one of the Accessibility Stewards can weigh in with their thoughts? @Qianqianye @calebfoss, @cosmicbhejafry, @apoorva-a98, @tedkmburu, @Zarkv, @SkylerW99, @itsjoopark, @hannahvy, @nhasalajoshi

aleannab · 2024-09-26T17:07:09Z

One last thought: as @aleannab mentioned, can we use the same two colors, (1a85ff and d41159), in the blendMode(SUBTRACT)? I believe you're currently using blue and red, correct?

Ah, I was using the same two colors (1a85ff and d41159) across the board. I agree that makes sense!

samarsrivastav · 2024-09-26T17:29:29Z

Certainly! I will ensure that the same colors are used consistently across all blend mode examples.

One final clarification I’d like to make, regarding the issue:

We are using the colors #1a85ff and #d41159 for the blend mode.
However, when describing them, we are referencing the color names from the table provided above.

@aleannab and @perminder-17, please give a thumbs up if you feel this is the approach, and I will proceed with committing the changes.

aleannab · 2024-09-26T17:48:53Z

Certainly! I will ensure that the same colors are used consistently across all blend mode examples.

One final clarification I’d like to make, regarding the issue:
1. We are using the colors #1a85ff and #d41159 for the blend mode.

2. However, when describing them, we are referencing the color names from the table provided above.
@aleannab and @perminder-17, please give a thumbs up if you feel this is the approach, and I will proceed with committing the changes.

Thanks @samarsrivastav ! Thumbs up to the first item! Let's wait on the color names for describe() until one of the Accessibility stewards can give their opinion.

perminder-17 · 2024-09-26T17:52:30Z

Thanks @samarsrivastav ! Thumbs up to the first item! Let's wait on the color names for describe() until one of the Accessibility stewards can give their opinion.

Sounds good!

davepagurek · 2024-09-26T20:47:31Z

Hi! I'm not an accessibility steward, but assuming the descriptions are intended for humans using screen readers, we probably should be using some kind of human-readable color in the description. Ideally we've picked input colors that make the visual differences between e.g. ADD and DODGE different enough that we can describe them as something like "blue" vs "lighter blue", but since that sounds hard to achieve across the board, I think it's ok if not all the described colours are distinct from each other.

perminder-17 · 2024-09-26T22:10:18Z

Thanks for the response @davepagurek

However, when describing them, we are referencing the color names from the table provided above.

I think you can go with the naming instead of the codes in order to describe them. @samarsrivastav .

Thanks for your work.

aleannab · 2024-09-26T23:22:37Z

Thank you @davepagurek and @perminder-17! If this helps, @samarsrivastav , I have the following table (with ChatGPT doing the heavy lifting). Feel free to make adjustments as you see fit. I gave ChatGPT the image I posted in the original issue.

Blend Mode	Colors	Overlapped
BLEND	Sky Blue (#1a85ff) and Deep Rose (#d41159)	no overlap
HARD_LIGHT	Ocean Blue (#25b7ff) and Hot Pink (#e6187e)	Magenta Purple (#b518b2)
BURN	Cobalt Blue (#006fb4) and Burgundy (#a50028)	Black (#.000028)
MULTIPLY	Slate Blue (#125eb4) and Plum (#96187e)	Dark Indigo (#0f063f)
DARKEST	Steel Blue (#1a85b4) and Cranberry (#b41159)	Deep Purple (#1a1159)
SOFT_LIGHT	Pale Sky (#8ab6d6) and Rose Blush (#cb86a4)	Lavender (#ab89cc)
LIGHTEST	Pale Lavender (#b4b4ff) and Soft Beige (#d4b4b4)	Pale Lilac (#d4b4ff)
SCREEN	Baby Blue (#bcdbff) and Peach Pink (#f2b9ce)	Misty Lilac (#f4ddff)
ADD	Icy Blue (#ceffff) and Light Lavender (#ffc5ff)	White (#ffffff)
DODGE	Aqua Blue (#c8ffff) and Light Pink (#ffc1ff)	White (#ffffff)
OVERLAY	Cornflower Blue (#78b7ff) and Light Rose (#e6739d)	Violet (#c879ff)
DIFFERENCE	Light Burgundy (#9a2f4b) and Forest Green (#20a35b)	Dark Cocoa (#3a1e0e)
EXCLUSION	Earthy Brown (#a97d4b) and Muted Sage (#5dad8f)	Sage Green (#647d70)
SUBTRACT	Burnt Orange (#ae4300) and Sea Green (#00b76f)	Forest Green (#.003200)
REMOVE	Transparent X
REPLACE	Only draws the last item drawn, line is Deep Rose (#d41159)

perminder-17 · 2024-09-26T23:37:51Z

Thank you @davepagurek and @perminder-17! If this helps, @samarsrivastav , I have the following table (with ChatGPT doing

Looks awesome. Thanks:)

samarsrivastav · 2024-09-27T14:25:31Z

@perminder-17 and @aleannab, I have committed the changes. Kindly review them and share your feedback at your earliest convenience.

aleannab

Thanks @samarsrivastav! This is extremely nit-picky, but there's inconsistency throughout regarding capitalization of color names. Following the format of the original example, all colors should be lowercase, unless it's at the start of a sentence. If @perminder-17 doesn't see anything else, I think we should be good to go!

aleannab · 2024-09-27T17:33:00Z

src/core/rendering.js

@@ -812,7 +812,7 @@ p5.prototype.clearDepth = function(depth) {
 *   stroke('#d41159');
 *   line(75, 25, 25, 75);
 *
- *   describe('A faint #1a85ff line and a faint #d41159 line form an X on a gray background. The area where they overlap is #B518B2.');
+ *   describe('An Ocean Blue line and a Hot pink line form an X on a gray background. The area where they overlap is Magenta purple.');


There is inconsistency in capitalization of color names (e.g. "Ocean Blue" vs. "Hot pink") here, and across all the examples.

perminder-17

all colors should be lowercase, unless it's at the start of a sentence

Good catch!

I think we are good to go....could be the last bit only. Thanks.

perminder-17 · 2024-09-27T17:54:36Z

src/core/rendering.js

@@ -1247,7 +1247,7 @@ p5.prototype.blendMode = function(mode) {
 *   // Draw the circle.
 *   circle(50, 50, 40);
 *
- *   describe('A fiery sun drawn on a light blue background.');
+ *   describe('A fiery sun drawn on a light #1a85ff background.');


Suggested change

* describe('A fiery sun drawn on a light #1a85ff background.');

* describe('A fiery sun drawn on a light blue background.');

Maybe the last bit. This change you have made on the drawingContext method. So this can be reverted to it's color name.

samarsrivastav · 2024-09-27T18:08:11Z

@perminder-17, I apologize for the oversight earlier. I've reverted to the original naming and ensured that all color names are now consistently in lowercase.

perminder-17

This is the last bit. Otherwise we are good to go!

src/core/rendering.js

Co-authored-by: Perminder Singh <[email protected]>

perminder-17

Looks good! Thanks for your work @samarsrivastav and thanks for the review @aleannab

davepagurek

Thanks everyone!

aleannab · 2024-09-27T20:30:20Z

Yay @samarsrivastav! And thank you @perminder-17 and @davepagurek!

Change Reference Example

5022f82

perminder-17 self-requested a review September 25, 2024 21:43

perminder-17 reviewed Sep 26, 2024

View reviewed changes

aleannab reviewed Sep 26, 2024

View reviewed changes

Change color and other fixes

2597dea

add describing message

a6a2b01

aleannab suggested changes Sep 27, 2024

View reviewed changes

perminder-17 reviewed Sep 27, 2024

View reviewed changes

changing the color name to lowercase

bb512f1

perminder-17 reviewed Sep 27, 2024

View reviewed changes

src/core/rendering.js Outdated Show resolved Hide resolved

Update src/core/rendering.js

92bc584

Co-authored-by: Perminder Singh <[email protected]>

perminder-17 requested a review from davepagurek September 27, 2024 19:02

perminder-17 approved these changes Sep 27, 2024

View reviewed changes

davepagurek approved these changes Sep 27, 2024

View reviewed changes

davepagurek merged commit 58a3c24 into processing:main Sep 27, 2024
2 checks passed

	* describe('A fiery sun drawn on a light #1a85ff background.');
	* describe('A fiery sun drawn on a light blue background.');

Change Reference Example #7280

Change Reference Example #7280

Conversation

samarsrivastav commented Sep 22, 2024

welcome bot commented Sep 22, 2024

Qianqianye commented Sep 25, 2024

perminder-17 left a comment • edited Loading

Choose a reason for hiding this comment

perminder-17 Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

samarsrivastav Sep 26, 2024

Choose a reason for hiding this comment

perminder-17 Sep 26, 2024

Choose a reason for hiding this comment

samarsrivastav Sep 26, 2024

Choose a reason for hiding this comment

aleannab Sep 26, 2024

Choose a reason for hiding this comment

perminder-17 Sep 26, 2024

Choose a reason for hiding this comment

aleannab Sep 26, 2024

Choose a reason for hiding this comment

samarsrivastav Sep 26, 2024

Choose a reason for hiding this comment

aleannab commented Sep 26, 2024 • edited Loading

perminder-17 commented Sep 26, 2024

aleannab commented Sep 26, 2024 • edited Loading

perminder-17 commented Sep 26, 2024

samarsrivastav commented Sep 26, 2024

samarsrivastav commented Sep 26, 2024

perminder-17 commented Sep 26, 2024

perminder-17 commented Sep 26, 2024

aleannab commented Sep 26, 2024

aleannab commented Sep 26, 2024

samarsrivastav commented Sep 26, 2024

aleannab commented Sep 26, 2024 • edited Loading

perminder-17 commented Sep 26, 2024 • edited Loading

davepagurek commented Sep 26, 2024

perminder-17 commented Sep 26, 2024

aleannab commented Sep 26, 2024 • edited Loading

perminder-17 commented Sep 26, 2024

samarsrivastav commented Sep 27, 2024

aleannab left a comment

Choose a reason for hiding this comment

aleannab Sep 27, 2024 • edited Loading

Choose a reason for hiding this comment

perminder-17 left a comment • edited Loading

Choose a reason for hiding this comment

perminder-17 Sep 27, 2024 • edited Loading

Choose a reason for hiding this comment

samarsrivastav commented Sep 27, 2024

perminder-17 left a comment

Choose a reason for hiding this comment

perminder-17 left a comment

Choose a reason for hiding this comment

davepagurek left a comment

Choose a reason for hiding this comment

aleannab commented Sep 27, 2024

perminder-17 left a comment •

edited

Loading

perminder-17 Sep 26, 2024 •

edited

Loading

aleannab commented Sep 26, 2024 •

edited

Loading

aleannab commented Sep 26, 2024 •

edited

Loading

aleannab commented Sep 26, 2024 •

edited

Loading

perminder-17 commented Sep 26, 2024 •

edited

Loading

aleannab commented Sep 26, 2024 •

edited

Loading

aleannab Sep 27, 2024 •

edited

Loading

perminder-17 left a comment •

edited

Loading

perminder-17 Sep 27, 2024 •

edited

Loading