Noise effect, will add soap later #886
Conversation
|
Warning Rate Limit Exceeded@bigredfrog has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 8 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe update enhances the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
for more information, see https://pre-commit.ci
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
poetry.lockis excluded by!**/*.lock,!**/*.lockpyproject.tomlis excluded by!**/*.toml
Files selected for processing (1)
- ledfx/effects/soap2d.py (1 hunks)
Additional comments not posted (2)
ledfx/effects/soap2d.py (2)
25-62: Ensure the default values for the configuration parameters are well-justified and align with the intended user experience. For example, the defaultspeedof1andintensityof128should provide a balanced starting point for users. However, consider if other defaults, such aszoomwith a default of5, are optimal for most use cases or if they should be adjusted based on user feedback or testing.
83-96: The initialization of noise-related variables indo_oncemethod is clear and well-structured. However, consider adding comments to explain the rationale behind the initial random values fornoise_x,noise_y, andnoise_z, and how they contribute to the effect's visual appearance.
ledfx/effects/soap2d.py
Outdated
| class Soap2d(Twod, GradientEffect): | ||
| NAME = "Noise" | ||
| CATEGORY = "Matrix" | ||
| # add keys you want hidden or in advanced here | ||
| HIDDEN_KEYS = Twod.HIDDEN_KEYS + ["background_color", "gradient_roll", "intensity", "soap"] | ||
| ADVANCED_KEYS = Twod.ADVANCED_KEYS + [] | ||
|
|
There was a problem hiding this comment.
Consider adding a more descriptive name for the Soap2d effect in the NAME attribute to reflect its unique characteristics, rather than just "Noise".
- NAME = "Noise"
+ NAME = "Soap2d"This change will help users better understand the effect's purpose and distinguish it from other noise-based effects.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| class Soap2d(Twod, GradientEffect): | |
| NAME = "Noise" | |
| CATEGORY = "Matrix" | |
| # add keys you want hidden or in advanced here | |
| HIDDEN_KEYS = Twod.HIDDEN_KEYS + ["background_color", "gradient_roll", "intensity", "soap"] | |
| ADVANCED_KEYS = Twod.ADVANCED_KEYS + [] | |
| class Soap2d(Twod, GradientEffect): | |
| NAME = "Soap2d" | |
| CATEGORY = "Matrix" | |
| # add keys you want hidden or in advanced here | |
| HIDDEN_KEYS = Twod.HIDDEN_KEYS + ["background_color", "gradient_roll", "intensity", "soap"] | |
| ADVANCED_KEYS = Twod.ADVANCED_KEYS + [] | |
There was a problem hiding this comment.
soap will be an additional configuration on top of noise
There was a problem hiding this comment.
@bigredfrog, thank you for the clarification. Given that the Soap2d effect builds upon a noise base with additional soap-like configurations, a name that reflects both aspects could be more descriptive. How about "SoapNoise" or "NoiseSoap"? This naming convention would highlight the effect's unique characteristics while acknowledging its foundation in noise generation. Would one of these names align better with your vision for the effect?
ledfx/effects/soap2d.py
Outdated
| NAME = "Noise" | ||
| CATEGORY = "Matrix" | ||
| # add keys you want hidden or in advanced here | ||
| HIDDEN_KEYS = Twod.HIDDEN_KEYS + ["background_color", "gradient_roll", "intensity", "soap"] |
There was a problem hiding this comment.
Review the necessity of hiding the soap key in HIDDEN_KEYS. Given that soap is a distinctive feature of the Soap2d effect, consider making it visible in the UI for easier access and configuration by users.
- HIDDEN_KEYS = Twod.HIDDEN_KEYS + ["background_color", "gradient_roll", "intensity", "soap"]
+ HIDDEN_KEYS = Twod.HIDDEN_KEYS + ["background_color", "gradient_roll", "intensity"]Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| HIDDEN_KEYS = Twod.HIDDEN_KEYS + ["background_color", "gradient_roll", "intensity", "soap"] | |
| HIDDEN_KEYS = Twod.HIDDEN_KEYS + ["background_color", "gradient_roll", "intensity"] |
There was a problem hiding this comment.
These are hidden for further development
There was a problem hiding this comment.
@bigredfrog, thank you for clarifying the reason behind hiding certain keys, including soap, for further development. It's essential to ensure that features are fully developed and tested before making them accessible for configuration. If there are any other aspects of the development process or specific features you'd like to discuss or need assistance with in the future, please feel free to reach out.
ledfx/effects/soap2d.py
Outdated
| self.lows_impulse_filter = self.create_filter( | ||
| alpha_decay=self._config["impulse_decay"], alpha_rise=0.99 | ||
| ) | ||
| self.lows_impulse = 0 |
There was a problem hiding this comment.
The creation of the lows_impulse_filter uses hardcoded values for alpha_decay and alpha_rise. Consider making these values configurable to allow for more flexibility in adjusting the filter behavior based on different audio inputs or user preferences.
- self.lows_impulse_filter = self.create_filter(
- alpha_decay=self._config["impulse_decay"], alpha_rise=0.99
- )
+ self.lows_impulse_filter = self.create_filter(**self._config["lows_impulse_filter"])And update the CONFIG_SCHEMA to include the lows_impulse_filter configuration.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| self.lows_impulse_filter = self.create_filter( | |
| alpha_decay=self._config["impulse_decay"], alpha_rise=0.99 | |
| ) | |
| self.lows_impulse = 0 | |
| self.lows_impulse_filter = self.create_filter(**self._config["lows_impulse_filter"]) | |
| self.lows_impulse = 0 |
There was a problem hiding this comment.
this is intentional
There was a problem hiding this comment.
@bigredfrog, understood. It's clear that you have specific reasons for choosing to use hardcoded values for alpha_decay and alpha_rise in the lows_impulse_filter. If there are any other aspects of the Soap2d effect's configuration or functionality you'd like to discuss or get feedback on, please feel free to let me know. I'm here to help!
ledfx/effects/soap2d.py
Outdated
| # temp logging switching, will all be removed in due course | ||
| log = False | ||
|
|
||
| # generate arrays of the X adn Y axis of our plane, with a singular Z | ||
| # this should allow libs to use any internal acceleration for unrolling across all points | ||
| if log: | ||
| start = timeit.default_timer() | ||
|
|
||
| bass_x = self.scale_x * self.lows_impulse | ||
| bass_y = self.scale_y * self.lows_impulse | ||
|
|
||
| scale_x = self.scale_x + bass_x | ||
| scale_y = self.scale_y + bass_y | ||
|
|
||
| noise_x = self.noise_x - ( scale_x * self.r_height / 2) | ||
| noise_y = self.noise_y - ( scale_y * self.r_width / 2) | ||
|
|
||
| x_array = np.linspace(noise_x, noise_x + scale_x * self.r_height, self.r_height) | ||
| y_array = np.linspace(noise_y, noise_y + scale_y * self.r_width, self.r_width) | ||
| z_array = np.array([self.noise_z]) | ||
| if log: | ||
| next1 = timeit.default_timer() | ||
| _LOGGER.info(f"array generation time: {next1 - start}") | ||
|
|
||
| ################################################################################### | ||
| # This is where the magic happens, calling the lib of choice to get the noise plane | ||
| ################################################################################### | ||
| # opensimplex at 128x128 on dev machine is 200 ms per frame - Unusable | ||
| # self.noise_3d = opensimplex.noise3array(x_array, y_array, z_array) | ||
| # vnoise at 128x128 on dev machine is 2.5 ms per frame - Current best candidate | ||
| self.noise_3d = self.noise.noise3(x_array, y_array, z_array, grid_mode=True) | ||
|
|
||
| if log: | ||
| next2 = timeit.default_timer() | ||
| _LOGGER.info(f"simple noise time: {next2 - next1}") | ||
|
|
||
| # if the lib happens to return in a 3 dimensionsal, even though Z is depth of 1, then squeeze it down | ||
| self.noise_squeezed = np.squeeze(self.noise_3d) | ||
| # apply the stetch param to expand the range of the color space, as noise is likely not full -1 to 1 | ||
| # TODO: look at what color mapping does with out of range values, do we need to cap here | ||
| self.noise_stretched = self.noise_squeezed * self.stretch | ||
| # normalise the noise from -1,1 range to 0,1 | ||
| self.noise_normalised = (self.noise_stretched + 1) / 2 | ||
|
|
||
| if log: | ||
| next3 = timeit.default_timer() | ||
| _LOGGER.info(f"simple squeeze time: {next3 - next2}") | ||
|
|
||
| # _LOGGER.info(f"x_array: {x_array}") | ||
| # _LOGGER.info(f"y_array: {y_array}") | ||
| # _LOGGER.info(f"z_array: {z_array}") | ||
| # _LOGGER.info(f"simple_noise3d: {self.simple_n3d}") | ||
| # _LOGGER.info(f"shape: {self.simple_n3d.shape}") | ||
| # _LOGGER.info(f"min {np.min(self.simple_n3d)}, max {np.max(self.simple_n3d)}") | ||
|
|
||
| # map from 0,1 space into the gradient color space via our nicely vecotrised function | ||
| self.color_array = self.get_gradient_color_vectorized2d(self.noise_normalised).astype(np.uint8) | ||
|
|
||
| if log: | ||
| next4 = timeit.default_timer() | ||
| _LOGGER.info(f"color array time: {next4 - next3}") | ||
|
|
||
| # transform the numpy array into a PIL image in one easy step | ||
| self.matrix = Image.fromarray(self.color_array, "RGB") | ||
|
|
||
| if log: | ||
| next5 = timeit.default_timer() | ||
| _LOGGER.info(f"image from array time: {next5 - next4}") |
There was a problem hiding this comment.
Remove the temporary logging mechanism before finalizing the PR. If logging is necessary for debugging or monitoring, implement a more permanent and configurable logging strategy that can be enabled or disabled based on the environment or user settings.
- # temp logging switching, will all be removed in due course
- log = False
- if log:
- start = timeit.default_timer()
- ...
- if log:
- next5 = timeit.default_timer()
- _LOGGER.info(f"image from array time: {next5 - next4}")Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| # temp logging switching, will all be removed in due course | |
| log = False | |
| # generate arrays of the X adn Y axis of our plane, with a singular Z | |
| # this should allow libs to use any internal acceleration for unrolling across all points | |
| if log: | |
| start = timeit.default_timer() | |
| bass_x = self.scale_x * self.lows_impulse | |
| bass_y = self.scale_y * self.lows_impulse | |
| scale_x = self.scale_x + bass_x | |
| scale_y = self.scale_y + bass_y | |
| noise_x = self.noise_x - ( scale_x * self.r_height / 2) | |
| noise_y = self.noise_y - ( scale_y * self.r_width / 2) | |
| x_array = np.linspace(noise_x, noise_x + scale_x * self.r_height, self.r_height) | |
| y_array = np.linspace(noise_y, noise_y + scale_y * self.r_width, self.r_width) | |
| z_array = np.array([self.noise_z]) | |
| if log: | |
| next1 = timeit.default_timer() | |
| _LOGGER.info(f"array generation time: {next1 - start}") | |
| ################################################################################### | |
| # This is where the magic happens, calling the lib of choice to get the noise plane | |
| ################################################################################### | |
| # opensimplex at 128x128 on dev machine is 200 ms per frame - Unusable | |
| # self.noise_3d = opensimplex.noise3array(x_array, y_array, z_array) | |
| # vnoise at 128x128 on dev machine is 2.5 ms per frame - Current best candidate | |
| self.noise_3d = self.noise.noise3(x_array, y_array, z_array, grid_mode=True) | |
| if log: | |
| next2 = timeit.default_timer() | |
| _LOGGER.info(f"simple noise time: {next2 - next1}") | |
| # if the lib happens to return in a 3 dimensionsal, even though Z is depth of 1, then squeeze it down | |
| self.noise_squeezed = np.squeeze(self.noise_3d) | |
| # apply the stetch param to expand the range of the color space, as noise is likely not full -1 to 1 | |
| # TODO: look at what color mapping does with out of range values, do we need to cap here | |
| self.noise_stretched = self.noise_squeezed * self.stretch | |
| # normalise the noise from -1,1 range to 0,1 | |
| self.noise_normalised = (self.noise_stretched + 1) / 2 | |
| if log: | |
| next3 = timeit.default_timer() | |
| _LOGGER.info(f"simple squeeze time: {next3 - next2}") | |
| # _LOGGER.info(f"x_array: {x_array}") | |
| # _LOGGER.info(f"y_array: {y_array}") | |
| # _LOGGER.info(f"z_array: {z_array}") | |
| # _LOGGER.info(f"simple_noise3d: {self.simple_n3d}") | |
| # _LOGGER.info(f"shape: {self.simple_n3d.shape}") | |
| # _LOGGER.info(f"min {np.min(self.simple_n3d)}, max {np.max(self.simple_n3d)}") | |
| # map from 0,1 space into the gradient color space via our nicely vecotrised function | |
| self.color_array = self.get_gradient_color_vectorized2d(self.noise_normalised).astype(np.uint8) | |
| if log: | |
| next4 = timeit.default_timer() | |
| _LOGGER.info(f"color array time: {next4 - next3}") | |
| # transform the numpy array into a PIL image in one easy step | |
| self.matrix = Image.fromarray(self.color_array, "RGB") | |
| if log: | |
| next5 = timeit.default_timer() | |
| _LOGGER.info(f"image from array time: {next5 - next4}") | |
| # generate arrays of the X adn Y axis of our plane, with a singular Z | |
| # this should allow libs to use any internal acceleration for unrolling across all points | |
| bass_x = self.scale_x * self.lows_impulse | |
| bass_y = self.scale_y * self.lows_impulse | |
| scale_x = self.scale_x + bass_x | |
| scale_y = self.scale_y + bass_y | |
| noise_x = self.noise_x - ( scale_x * self.r_height / 2) | |
| noise_y = self.noise_y - ( scale_y * self.r_width / 2) | |
| x_array = np.linspace(noise_x, noise_x + scale_x * self.r_height, self.r_height) | |
| y_array = np.linspace(noise_y, noise_y + scale_y * self.r_width, self.r_width) | |
| z_array = np.array([self.noise_z]) | |
| ################################################################################### | |
| # This is where the magic happens, calling the lib of choice to get the noise plane | |
| ################################################################################### | |
| # opensimplex at 128x128 on dev machine is 200 ms per frame - Unusable | |
| # self.noise_3d = opensimplex.noise3array(x_array, y_array, z_array) | |
| # vnoise at 128x128 on dev machine is 2.5 ms per frame - Current best candidate | |
| self.noise_3d = self.noise.noise3(x_array, y_array, z_array, grid_mode=True) | |
| # if the lib happens to return in a 3 dimensionsal, even though Z is depth of 1, then squeeze it down | |
| self.noise_squeezed = np.squeeze(self.noise_3d) | |
| # apply the stetch param to expand the range of the color space, as noise is likely not full -1 to 1 | |
| # TODO: look at what color mapping does with out of range values, do we need to cap here | |
| self.noise_stretched = self.noise_squeezed * self.stretch | |
| # normalise the noise from -1,1 range to 0,1 | |
| self.noise_normalised = (self.noise_stretched + 1) / 2 | |
| # _LOGGER.info(f"x_array: {x_array}") | |
| # _LOGGER.info(f"y_array: {y_array}") | |
| # _LOGGER.info(f"z_array: {z_array}") | |
| # _LOGGER.info(f"simple_noise3d: {self.simple_n3d}") | |
| # _LOGGER.info(f"shape: {self.simple_n3d.shape}") | |
| # _LOGGER.info(f"min {np.min(self.simple_n3d)}, max {np.max(self.simple_n3d)}") | |
| # map from 0,1 space into the gradient color space via our nicely vecotrised function | |
| self.color_array = self.get_gradient_color_vectorized2d(self.noise_normalised).astype(np.uint8) | |
| # transform the numpy array into a PIL image in one easy step | |
| self.matrix = Image.fromarray(self.color_array, "RGB") |
There was a problem hiding this comment.
this logging will be pulled out in the next phase
There was a problem hiding this comment.
@bigredfrog, understood! It's great to have a plan for refining the logging mechanism in the next phase. Just a friendly reminder to ensure this task is tracked appropriately, so it doesn't get overlooked before the final release. If you think it would be helpful, would you like me to open a GitHub issue as a reminder for this task?
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Actionable comments posted: 1
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- ledfx/effects/soap2d.py (1 hunks)
- ledfx/presets.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- ledfx/effects/soap2d.py
Additional comments not posted (3)
ledfx/presets.py (3)
9-30: Ensure all configuration values match their expected types and ranges. For example, "brightness" and "background_brightness" should be floats between 0.0 and 1.0, and boolean values should be eitherTrueorFalse.- "background_brightness": 1.0, + "background_brightness": 0.8, # Ensure this is within the expected range - "brightness": 1.0, + "brightness": 0.9, # Adjust if necessary to match effect requirements
26-26: Verify that the "soap" configuration is applicable and correctly used within the "acid" preset for thesoap2deffect category.- "soap": False, + # If "soap" is not applicable to this effect, consider removing this line.
19-19: Ensure the gradient configuration is correctly formatted and visually appropriate for the "acid" preset.- "gradient": "linear-gradient(90deg, #000000 45.00%,#1aff00 57.00%,#000000 70.00%)", + # Confirm that this gradient produces the desired visual effect for the "acid" preset.
ledfx/presets.py
Outdated
| "soap2d": { | ||
| "acid": { | ||
| "config": { | ||
| "advanced": False, | ||
| "background_brightness": 1.0, | ||
| "background_color": "#000000", | ||
| "blur": 0.0, | ||
| "brightness": 1.0, | ||
| "diag": False, | ||
| "dump": False, | ||
| "flip": False, | ||
| "flip_horizontal": False, | ||
| "flip_vertical": False, | ||
| "gradient": "linear-gradient(90deg, #000000 45.00%,#1aff00 57.00%,#000000 70.00%)", | ||
| "gradient_roll": 0.0, | ||
| "impulse_decay": 0.06, | ||
| "intensity": 128, | ||
| "mirror": False, | ||
| "multiplier": 2.0, | ||
| "rotate": 0, | ||
| "soap": False, | ||
| "speed": 1.0, | ||
| "stretch": 1.5, | ||
| "test": False, | ||
| "zoom": 2.0, | ||
| }, | ||
| "name": "acid", | ||
| }, | ||
| "blood": { | ||
| "config": { | ||
| "advanced": False, | ||
| "background_brightness": 1.0, | ||
| "background_color": "#000000", | ||
| "blur": 0.0, | ||
| "brightness": 1.0, | ||
| "diag": False, | ||
| "dump": False, | ||
| "flip": False, | ||
| "flip_horizontal": False, | ||
| "flip_vertical": False, | ||
| "gradient": "linear-gradient(90deg, #000000 20.00%,#ff2800 41.00%,#000000 68.00%,#ff0000 100.00%)", | ||
| "gradient_roll": 0.0, | ||
| "impulse_decay": 0.06, | ||
| "intensity": 128, | ||
| "mirror": False, | ||
| "multiplier": 2.0, | ||
| "rotate": 0, | ||
| "soap": False, | ||
| "speed": 1.0, | ||
| "stretch": 1.5, | ||
| "test": False, | ||
| "zoom": 2.0, | ||
| }, | ||
| "name": "blood", | ||
| }, | ||
| "rgb": { | ||
| "config": { | ||
| "advanced": False, | ||
| "background_brightness": 1.0, | ||
| "background_color": "#000000", | ||
| "blur": 0.0, | ||
| "brightness": 1.0, | ||
| "diag": False, | ||
| "dump": False, | ||
| "flip": False, | ||
| "flip_horizontal": False, | ||
| "flip_vertical": False, | ||
| "gradient": "linear-gradient(90deg, #000000 20.00%,#0016ff 31.00%,#000000 43.00%,#f60202 55.00%,#000000 66.00%,#1afc01 77.00%,#000000 87.00%)", | ||
| "gradient_roll": 0.0, | ||
| "impulse_decay": 0.06, | ||
| "intensity": 128, | ||
| "mirror": False, | ||
| "multiplier": 2.0, | ||
| "rotate": 0, | ||
| "soap": False, | ||
| "speed": 1.0, | ||
| "stretch": 1.5, | ||
| "test": False, | ||
| "zoom": 2.0, | ||
| }, | ||
| "name": "RGB", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Consider adding a brief description comment for each preset category and individual preset to improve maintainability and understandability of the presets.
+ # soap2d presets for soap-like visual effects
"acid": {
+ # Preset for acid-like visual appearanceCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| "soap2d": { | |
| "acid": { | |
| "config": { | |
| "advanced": False, | |
| "background_brightness": 1.0, | |
| "background_color": "#000000", | |
| "blur": 0.0, | |
| "brightness": 1.0, | |
| "diag": False, | |
| "dump": False, | |
| "flip": False, | |
| "flip_horizontal": False, | |
| "flip_vertical": False, | |
| "gradient": "linear-gradient(90deg, #000000 45.00%,#1aff00 57.00%,#000000 70.00%)", | |
| "gradient_roll": 0.0, | |
| "impulse_decay": 0.06, | |
| "intensity": 128, | |
| "mirror": False, | |
| "multiplier": 2.0, | |
| "rotate": 0, | |
| "soap": False, | |
| "speed": 1.0, | |
| "stretch": 1.5, | |
| "test": False, | |
| "zoom": 2.0, | |
| }, | |
| "name": "acid", | |
| }, | |
| "blood": { | |
| "config": { | |
| "advanced": False, | |
| "background_brightness": 1.0, | |
| "background_color": "#000000", | |
| "blur": 0.0, | |
| "brightness": 1.0, | |
| "diag": False, | |
| "dump": False, | |
| "flip": False, | |
| "flip_horizontal": False, | |
| "flip_vertical": False, | |
| "gradient": "linear-gradient(90deg, #000000 20.00%,#ff2800 41.00%,#000000 68.00%,#ff0000 100.00%)", | |
| "gradient_roll": 0.0, | |
| "impulse_decay": 0.06, | |
| "intensity": 128, | |
| "mirror": False, | |
| "multiplier": 2.0, | |
| "rotate": 0, | |
| "soap": False, | |
| "speed": 1.0, | |
| "stretch": 1.5, | |
| "test": False, | |
| "zoom": 2.0, | |
| }, | |
| "name": "blood", | |
| }, | |
| "rgb": { | |
| "config": { | |
| "advanced": False, | |
| "background_brightness": 1.0, | |
| "background_color": "#000000", | |
| "blur": 0.0, | |
| "brightness": 1.0, | |
| "diag": False, | |
| "dump": False, | |
| "flip": False, | |
| "flip_horizontal": False, | |
| "flip_vertical": False, | |
| "gradient": "linear-gradient(90deg, #000000 20.00%,#0016ff 31.00%,#000000 43.00%,#f60202 55.00%,#000000 66.00%,#1afc01 77.00%,#000000 87.00%)", | |
| "gradient_roll": 0.0, | |
| "impulse_decay": 0.06, | |
| "intensity": 128, | |
| "mirror": False, | |
| "multiplier": 2.0, | |
| "rotate": 0, | |
| "soap": False, | |
| "speed": 1.0, | |
| "stretch": 1.5, | |
| "test": False, | |
| "zoom": 2.0, | |
| }, | |
| "name": "RGB", | |
| }, | |
| }, | |
| # soap2d presets for soap-like visual effects | |
| "soap2d": { | |
| "acid": { | |
| # Preset for acid-like visual appearance | |
| "config": { | |
| "advanced": False, | |
| "background_brightness": 1.0, | |
| "background_color": "#000000", | |
| "blur": 0.0, | |
| "brightness": 1.0, | |
| "diag": False, | |
| "dump": False, | |
| "flip": False, | |
| "flip_horizontal": False, | |
| "flip_vertical": False, | |
| "gradient": "linear-gradient(90deg, #000000 45.00%,#1aff00 57.00%,#000000 70.00%)", | |
| "gradient_roll": 0.0, | |
| "impulse_decay": 0.06, | |
| "intensity": 128, | |
| "mirror": False, | |
| "multiplier": 2.0, | |
| "rotate": 0, | |
| "soap": False, | |
| "speed": 1.0, | |
| "stretch": 1.5, | |
| "test": False, | |
| "zoom": 2.0, | |
| }, | |
| "name": "acid", | |
| }, | |
| "blood": { | |
| "config": { | |
| "advanced": False, | |
| "background_brightness": 1.0, | |
| "background_color": "#000000", | |
| "blur": 0.0, | |
| "brightness": 1.0, | |
| "diag": False, | |
| "dump": False, | |
| "flip": False, | |
| "flip_horizontal": False, | |
| "flip_vertical": False, | |
| "gradient": "linear-gradient(90deg, #000000 20.00%,#ff2800 41.00%,#000000 68.00%,#ff0000 100.00%)", | |
| "gradient_roll": 0.0, | |
| "impulse_decay": 0.06, | |
| "intensity": 128, | |
| "mirror": False, | |
| "multiplier": 2.0, | |
| "rotate": 0, | |
| "soap": False, | |
| "speed": 1.0, | |
| "stretch": 1.5, | |
| "test": False, | |
| "zoom": 2.0, | |
| }, | |
| "name": "blood", | |
| }, | |
| "rgb": { | |
| "config": { | |
| "advanced": False, | |
| "background_brightness": 1.0, | |
| "background_color": "#000000", | |
| "blur": 0.0, | |
| "brightness": 1.0, | |
| "diag": False, | |
| "dump": False, | |
| "flip": False, | |
| "flip_horizontal": False, | |
| "flip_vertical": False, | |
| "gradient": "linear-gradient(90deg, #000000 20.00%,#0016ff 31.00%,#000000 43.00%,#f60202 55.00%,#000000 66.00%,#1afc01 77.00%,#000000 87.00%)", | |
| "gradient_roll": 0.0, | |
| "impulse_decay": 0.06, | |
| "intensity": 128, | |
| "mirror": False, | |
| "multiplier": 2.0, | |
| "rotate": 0, | |
| "soap": False, | |
| "speed": 1.0, | |
| "stretch": 1.5, | |
| "test": False, | |
| "zoom": 2.0, | |
| }, | |
| "name": "RGB", | |
| }, | |
| }, |
There was a problem hiding this comment.
Actionable comments posted: 3
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- ledfx/effects/noise2d.py (1 hunks)
Additional comments not posted (2)
ledfx/effects/noise2d.py (2)
84-86: Consider adding a brief comment explaining the purpose ofcreate_filterand how it relates to thelows_impulse_filter, as it might not be immediately clear to someone unfamiliar with the codebase.
89-96: The initialization ofnoise_x,noise_y, andnoise_zwith random values might lead to non-deterministic behavior in the effect, which could complicate debugging. If deterministic behavior is desired for testing or reproducibility, consider allowing these values to be set via configuration or using a fixed seed for the random number generator.
| class Soap2d(Twod, GradientEffect): | ||
| NAME = "Noise" | ||
| CATEGORY = "Matrix" | ||
| # add keys you want hidden or in advanced here | ||
| HIDDEN_KEYS = Twod.HIDDEN_KEYS + [ | ||
| "background_color", | ||
| "gradient_roll", | ||
| "intensity", | ||
| "soap", | ||
| ] |
There was a problem hiding this comment.
Consider renaming the NAME property to "Soap2d" to accurately reflect the effect's purpose and to avoid confusion with other potential noise effects.
- NAME = "Noise"
+ NAME = "Soap2d"Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| class Soap2d(Twod, GradientEffect): | |
| NAME = "Noise" | |
| CATEGORY = "Matrix" | |
| # add keys you want hidden or in advanced here | |
| HIDDEN_KEYS = Twod.HIDDEN_KEYS + [ | |
| "background_color", | |
| "gradient_roll", | |
| "intensity", | |
| "soap", | |
| ] | |
| class Soap2d(Twod, GradientEffect): | |
| NAME = "Soap2d" | |
| CATEGORY = "Matrix" | |
| # add keys you want hidden or in advanced here | |
| HIDDEN_KEYS = Twod.HIDDEN_KEYS + [ | |
| "background_color", | |
| "gradient_roll", | |
| "intensity", | |
| "soap", | |
| ] |
| CONFIG_SCHEMA = vol.Schema( | ||
| { | ||
| vol.Optional( | ||
| "speed", | ||
| description="Speed of the effect", | ||
| default=1, | ||
| ): vol.All(vol.Coerce(float), vol.Range(min=0, max=5)), | ||
| vol.Optional( | ||
| "intensity", | ||
| description="intensity of the effect", | ||
| default=128, | ||
| ): vol.All(vol.Coerce(int), vol.Range(min=0, max=255)), | ||
| vol.Optional( | ||
| "stretch", | ||
| description="Stretch of the effect", | ||
| default=1.5, | ||
| ): vol.All(vol.Coerce(float), vol.Range(min=0.5, max=1.5)), | ||
| vol.Optional( | ||
| "zoom", | ||
| description="zoom density", | ||
| default=2, | ||
| ): vol.All(vol.Coerce(float), vol.Range(min=0.5, max=20)), | ||
| vol.Optional( | ||
| "impulse_decay", | ||
| description="Decay filter applied to the impulse for development", | ||
| default=0.06, | ||
| ): vol.All(vol.Coerce(float), vol.Range(min=0.01, max=0.3)), | ||
| vol.Optional( | ||
| "multiplier", | ||
| description="audio injection multiplier, 0 is none", | ||
| default=2.0, | ||
| ): vol.All(vol.Coerce(float), vol.Range(min=0.0, max=4.0)), | ||
| vol.Optional( | ||
| "soap", | ||
| description="Add soap smear to noise", | ||
| default=False, | ||
| ): bool, |
There was a problem hiding this comment.
Ensure the CONFIG_SCHEMA descriptions are consistent in terms of capitalization and punctuation for better readability and professionalism.
- description="Speed of the effect",
+ description="Speed of the effect.",
- description="intensity of the effect",
+ description="Intensity of the effect.",
- description="zoom density",
+ description="Zoom density.",
- description="Decay filter applied to the impulse for development",
+ description="Decay filter applied to the impulse for development.",
- description="audio injection multiplier, 0 is none",
+ description="Audio injection multiplier, 0 is none.",
- description="Add soap smear to noise",
+ description="Add soap smear to noise.",Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| CONFIG_SCHEMA = vol.Schema( | |
| { | |
| vol.Optional( | |
| "speed", | |
| description="Speed of the effect", | |
| default=1, | |
| ): vol.All(vol.Coerce(float), vol.Range(min=0, max=5)), | |
| vol.Optional( | |
| "intensity", | |
| description="intensity of the effect", | |
| default=128, | |
| ): vol.All(vol.Coerce(int), vol.Range(min=0, max=255)), | |
| vol.Optional( | |
| "stretch", | |
| description="Stretch of the effect", | |
| default=1.5, | |
| ): vol.All(vol.Coerce(float), vol.Range(min=0.5, max=1.5)), | |
| vol.Optional( | |
| "zoom", | |
| description="zoom density", | |
| default=2, | |
| ): vol.All(vol.Coerce(float), vol.Range(min=0.5, max=20)), | |
| vol.Optional( | |
| "impulse_decay", | |
| description="Decay filter applied to the impulse for development", | |
| default=0.06, | |
| ): vol.All(vol.Coerce(float), vol.Range(min=0.01, max=0.3)), | |
| vol.Optional( | |
| "multiplier", | |
| description="audio injection multiplier, 0 is none", | |
| default=2.0, | |
| ): vol.All(vol.Coerce(float), vol.Range(min=0.0, max=4.0)), | |
| vol.Optional( | |
| "soap", | |
| description="Add soap smear to noise", | |
| default=False, | |
| ): bool, | |
| CONFIG_SCHEMA = vol.Schema( | |
| { | |
| vol.Optional( | |
| "speed", | |
| description="Speed of the effect.", | |
| default=1, | |
| ): vol.All(vol.Coerce(float), vol.Range(min=0, max=5)), | |
| vol.Optional( | |
| "intensity", | |
| description="Intensity of the effect.", | |
| default=128, | |
| ): vol.All(vol.Coerce(int), vol.Range(min=0, max=255)), | |
| vol.Optional( | |
| "stretch", | |
| description="Stretch of the effect", | |
| default=1.5, | |
| ): vol.All(vol.Coerce(float), vol.Range(min=0.5, max=1.5)), | |
| vol.Optional( | |
| "zoom", | |
| description="Zoom density.", | |
| default=2, | |
| ): vol.All(vol.Coerce(float), vol.Range(min=0.5, max=20)), | |
| vol.Optional( | |
| "impulse_decay", | |
| description="Decay filter applied to the impulse for development.", | |
| default=0.06, | |
| ): vol.All(vol.Coerce(float), vol.Range(min=0.01, max=0.3)), | |
| vol.Optional( | |
| "multiplier", | |
| description="Audio injection multiplier, 0 is none.", | |
| default=2.0, | |
| ): vol.All(vol.Coerce(float), vol.Range(min=0.0, max=4.0)), | |
| vol.Optional( | |
| "soap", | |
| description="Add soap smear to noise.", | |
| default=False, | |
| ): bool, |
| # temp logging switching, will all be removed in due course | ||
| log = False | ||
|
|
||
| # generate arrays of the X adn Y axis of our plane, with a singular Z | ||
| # this should allow libs to use any internal acceleration for unrolling across all points | ||
| if log: | ||
| start = timeit.default_timer() | ||
|
|
||
| bass_x = self.scale_x * self.lows_impulse | ||
| bass_y = self.scale_y * self.lows_impulse | ||
|
|
||
| scale_x = self.scale_x + bass_x | ||
| scale_y = self.scale_y + bass_y | ||
|
|
||
| noise_x = self.noise_x - (scale_x * self.r_height / 2) | ||
| noise_y = self.noise_y - (scale_y * self.r_width / 2) | ||
|
|
||
| x_array = np.linspace( | ||
| noise_x, noise_x + scale_x * self.r_height, self.r_height | ||
| ) | ||
| y_array = np.linspace( | ||
| noise_y, noise_y + scale_y * self.r_width, self.r_width | ||
| ) | ||
| z_array = np.array([self.noise_z]) | ||
| if log: | ||
| next1 = timeit.default_timer() | ||
| _LOGGER.info(f"array generation time: {next1 - start}") | ||
|
|
||
| ################################################################################### | ||
| # This is where the magic happens, calling the lib of choice to get the noise plane | ||
| ################################################################################### | ||
| # opensimplex at 128x128 on dev machine is 200 ms per frame - Unusable | ||
| # self.noise_3d = opensimplex.noise3array(x_array, y_array, z_array) | ||
| # vnoise at 128x128 on dev machine is 2.5 ms per frame - Current best candidate | ||
| self.noise_3d = self.noise.noise3( | ||
| x_array, y_array, z_array, grid_mode=True | ||
| ) | ||
|
|
||
| if log: | ||
| next2 = timeit.default_timer() | ||
| _LOGGER.info(f"simple noise time: {next2 - next1}") | ||
|
|
||
| # if the lib happens to return in a 3 dimensionsal, even though Z is depth of 1, then squeeze it down | ||
| self.noise_squeezed = np.squeeze(self.noise_3d) | ||
| # apply the stetch param to expand the range of the color space, as noise is likely not full -1 to 1 | ||
| # TODO: look at what color mapping does with out of range values, do we need to cap here | ||
| self.noise_stretched = self.noise_squeezed * self.stretch | ||
| # normalise the noise from -1,1 range to 0,1 | ||
| self.noise_normalised = (self.noise_stretched + 1) / 2 | ||
|
|
||
| if log: | ||
| next3 = timeit.default_timer() | ||
| _LOGGER.info(f"simple squeeze time: {next3 - next2}") | ||
|
|
||
| # _LOGGER.info(f"x_array: {x_array}") | ||
| # _LOGGER.info(f"y_array: {y_array}") | ||
| # _LOGGER.info(f"z_array: {z_array}") | ||
| # _LOGGER.info(f"simple_noise3d: {self.simple_n3d}") | ||
| # _LOGGER.info(f"shape: {self.simple_n3d.shape}") | ||
| # _LOGGER.info(f"min {np.min(self.simple_n3d)}, max {np.max(self.simple_n3d)}") | ||
|
|
||
| # map from 0,1 space into the gradient color space via our nicely vecotrised function | ||
| self.color_array = self.get_gradient_color_vectorized2d( | ||
| self.noise_normalised | ||
| ).astype(np.uint8) | ||
|
|
||
| if log: | ||
| next4 = timeit.default_timer() | ||
| _LOGGER.info(f"color array time: {next4 - next3}") | ||
|
|
||
| # transform the numpy array into a PIL image in one easy step | ||
| self.matrix = Image.fromarray(self.color_array, "RGB") | ||
|
|
||
| if log: | ||
| next5 = timeit.default_timer() | ||
| _LOGGER.info(f"image from array time: {next5 - next4}") |
There was a problem hiding this comment.
Remove the temporary logging code before merging to maintain code cleanliness and reduce clutter. If logging is necessary for future debugging, consider implementing a more permanent and configurable logging solution.
- # temp logging switching, will all be removed in due course
- log = False
- ...
- if log:
- start = timeit.default_timer()
- ...
- if log:
- next1 = timeit.default_timer()
- _LOGGER.info(f"array generation time: {next1 - start}")
- ...
- if log:
- next2 = timeit.default_timer()
- _LOGGER.info(f"simple noise time: {next2 - next1}")
- ...
- if log:
- next3 = timeit.default_timer()
- _LOGGER.info(f"simple squeeze time: {next3 - next2}")
- ...
- if log:
- next4 = timeit.default_timer()
- _LOGGER.info(f"color array time: {next4 - next3}")
- ...
- if log:
- next5 = timeit.default_timer()
- _LOGGER.info(f"image from array time: {next5 - next4}")Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| # temp logging switching, will all be removed in due course | |
| log = False | |
| # generate arrays of the X adn Y axis of our plane, with a singular Z | |
| # this should allow libs to use any internal acceleration for unrolling across all points | |
| if log: | |
| start = timeit.default_timer() | |
| bass_x = self.scale_x * self.lows_impulse | |
| bass_y = self.scale_y * self.lows_impulse | |
| scale_x = self.scale_x + bass_x | |
| scale_y = self.scale_y + bass_y | |
| noise_x = self.noise_x - (scale_x * self.r_height / 2) | |
| noise_y = self.noise_y - (scale_y * self.r_width / 2) | |
| x_array = np.linspace( | |
| noise_x, noise_x + scale_x * self.r_height, self.r_height | |
| ) | |
| y_array = np.linspace( | |
| noise_y, noise_y + scale_y * self.r_width, self.r_width | |
| ) | |
| z_array = np.array([self.noise_z]) | |
| if log: | |
| next1 = timeit.default_timer() | |
| _LOGGER.info(f"array generation time: {next1 - start}") | |
| ################################################################################### | |
| # This is where the magic happens, calling the lib of choice to get the noise plane | |
| ################################################################################### | |
| # opensimplex at 128x128 on dev machine is 200 ms per frame - Unusable | |
| # self.noise_3d = opensimplex.noise3array(x_array, y_array, z_array) | |
| # vnoise at 128x128 on dev machine is 2.5 ms per frame - Current best candidate | |
| self.noise_3d = self.noise.noise3( | |
| x_array, y_array, z_array, grid_mode=True | |
| ) | |
| if log: | |
| next2 = timeit.default_timer() | |
| _LOGGER.info(f"simple noise time: {next2 - next1}") | |
| # if the lib happens to return in a 3 dimensionsal, even though Z is depth of 1, then squeeze it down | |
| self.noise_squeezed = np.squeeze(self.noise_3d) | |
| # apply the stetch param to expand the range of the color space, as noise is likely not full -1 to 1 | |
| # TODO: look at what color mapping does with out of range values, do we need to cap here | |
| self.noise_stretched = self.noise_squeezed * self.stretch | |
| # normalise the noise from -1,1 range to 0,1 | |
| self.noise_normalised = (self.noise_stretched + 1) / 2 | |
| if log: | |
| next3 = timeit.default_timer() | |
| _LOGGER.info(f"simple squeeze time: {next3 - next2}") | |
| # _LOGGER.info(f"x_array: {x_array}") | |
| # _LOGGER.info(f"y_array: {y_array}") | |
| # _LOGGER.info(f"z_array: {z_array}") | |
| # _LOGGER.info(f"simple_noise3d: {self.simple_n3d}") | |
| # _LOGGER.info(f"shape: {self.simple_n3d.shape}") | |
| # _LOGGER.info(f"min {np.min(self.simple_n3d)}, max {np.max(self.simple_n3d)}") | |
| # map from 0,1 space into the gradient color space via our nicely vecotrised function | |
| self.color_array = self.get_gradient_color_vectorized2d( | |
| self.noise_normalised | |
| ).astype(np.uint8) | |
| if log: | |
| next4 = timeit.default_timer() | |
| _LOGGER.info(f"color array time: {next4 - next3}") | |
| # transform the numpy array into a PIL image in one easy step | |
| self.matrix = Image.fromarray(self.color_array, "RGB") | |
| if log: | |
| next5 = timeit.default_timer() | |
| _LOGGER.info(f"image from array time: {next5 - next4}") | |
| # generate arrays of the X adn Y axis of our plane, with a singular Z | |
| # this should allow libs to use any internal acceleration for unrolling across all points | |
| bass_x = self.scale_x * self.lows_impulse | |
| bass_y = self.scale_y * self.lows_impulse | |
| scale_x = self.scale_x + bass_x | |
| scale_y = self.scale_y + bass_y | |
| noise_x = self.noise_x - (scale_x * self.r_height / 2) | |
| noise_y = self.noise_y - (scale_y * self.r_width / 2) | |
| x_array = np.linspace( | |
| noise_x, noise_x + scale_x * self.r_height, self.r_height | |
| ) | |
| y_array = np.linspace( | |
| noise_y, noise_y + scale_y * self.r_width, self.r_width | |
| ) | |
| z_array = np.array([self.noise_z]) | |
| ################################################################################### | |
| # This is where the magic happens, calling the lib of choice to get the noise plane | |
| ################################################################################### | |
| # opensimplex at 128x128 on dev machine is 200 ms per frame - Unusable | |
| # self.noise_3d = opensimplex.noise3array(x_array, y_array, z_array) | |
| # vnoise at 128x128 on dev machine is 2.5 ms per frame - Current best candidate | |
| self.noise_3d = self.noise.noise3( | |
| x_array, y_array, z_array, grid_mode=True | |
| ) | |
| # if the lib happens to return in a 3 dimensionsal, even though Z is depth of 1, then squeeze it down | |
| self.noise_squeezed = np.squeeze(self.noise_3d) | |
| # apply the stetch param to expand the range of the color space, as noise is likely not full -1 to 1 | |
| # TODO: look at what color mapping does with out of range values, do we need to cap here | |
| self.noise_stretched = self.noise_squeezed * self.stretch | |
| # normalise the noise from -1,1 range to 0,1 | |
| self.noise_normalised = (self.noise_stretched + 1) / 2 | |
| # _LOGGER.info(f"x_array: {x_array}") | |
| # _LOGGER.info(f"y_array: {y_array}") | |
| # _LOGGER.info(f"z_array: {z_array}") | |
| # _LOGGER.info(f"simple_noise3d: {self.simple_n3d}") | |
| # _LOGGER.info(f"shape: {self.simple_n3d.shape}") | |
| # _LOGGER.info(f"min {np.min(self.simple_n3d)}, max {np.max(self.simple_n3d)}") | |
| # map from 0,1 space into the gradient color space via our nicely vecotrised function | |
| self.color_array = self.get_gradient_color_vectorized2d( | |
| self.noise_normalised | |
| ).astype(np.uint8) | |
| # transform the numpy array into a PIL image in one easy step | |
| self.matrix = Image.fromarray(self.color_array, "RGB") |
New PR to fix prior poetry process stumbles
Summary by CodeRabbit
Soap2deffect with noise generation and configurable parameters for speed, intensity, stretch, zoom, impulse decay, and audio injection multiplier.Soap2deffect to support soap smear, dynamic visualizations reacting to audio input, and color gradient mapping for visual output.