-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Add logging for experimental features #9109
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
base: main
Are you sure you want to change the base?
Changes from all commits
3481e9e
cd8b247
3f98421
d731d08
44de4c4
8d8c0b6
0b588fd
5ec15ec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| import { FES } from '../friendly_errors/fes'; | ||
|
|
||
|
|
||
| /* | ||
| * Sometimes p5.js includes experimental functionality whose APIs may | ||
| * change in the future, but for which we want more community feedback | ||
| * and testing. To be able to include these in a release, we need to: | ||
| * - Create a name for the subject area, e.g. 'webgpu' | ||
| * - Write a document in the contributor_docs folder for the subject area | ||
| * describing its goals and what we want feedback on. The file should match | ||
| * the subject area name, plus the .md suffix. | ||
| * - Write a message below that will show up in the console when functionality | ||
| * from that subject area. A link to the doc will be automatically appended. Index | ||
| * the message by the same subject area name. | ||
| * - Mark functions in that subject area with the experimental decorator, passing in | ||
| * the subject area name as a parameter to markExperimental. e.g.: | ||
| * p5.registerDecorator( | ||
| * 'p5.prototype.buildComputeShader', | ||
| * markExperimental('webgpu', p5) | ||
| * ) | ||
| * | ||
| * If overriding a method on a class, additionally pass in a function to get to the | ||
| * p5 instance from the class, e.g.: | ||
| * | ||
| * p5.registerDecorator( | ||
| * 'p5.Shader.prototype.modify', | ||
| * markExperimental('p5.strands', p5, (shader) => shader._renderer?._pInst) | ||
| * ) | ||
| * | ||
| * ...or, if you need to conditionally warn about experimental functionality, you | ||
| * can directly call warnExperimental(p5, pInst, subjectArea) inside a function. | ||
| */ | ||
|
|
||
| const experimentalMessages = { | ||
| webgpu: 'WEBGPU mode is experimental. Your feedback will help direct its development!', | ||
| 'p5.strands': 'p5.strands shaders are experimental. Your feedback will help shape its future!', | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Liekwise "p5.strands API (build shader functions, the hooks in them, and related constants) may change in future versions. You can get involved by giving feedback to help shape its future!" |
||
| }; | ||
|
|
||
| // Just in case it's not possible to get access to the p5 instance from something, | ||
| // we still don't want to make logs super noisy from repeated warnings, so we'll | ||
| // fall back on this global cache. It means if you create a second p5 instance, it | ||
| // wouldn't log again, but this is only here to handle edge case classes disconnected | ||
| // from the p5 instance anyway. | ||
| const globalWarningTarget = {}; | ||
|
|
||
| export function warnExperimental(p5, pInst, subjectArea) { | ||
| const target = pInst || globalWarningTarget; | ||
| if (!p5.disableFriendlyErrors && !target.warnedExperimental?.[subjectArea]) { | ||
| target.warnedExperimental = target.warnedExperimental || {}; | ||
| target.warnedExperimental[subjectArea] = true; | ||
|
|
||
| FES.log`${experimentalMessages[subjectArea]} For more info, see https://p5js.org/contribute/${subjectArea}/`(); | ||
| } | ||
| } | ||
|
|
||
| export function markExperimental(subjectArea, p5, getPInst = (targetObj) => targetObj) { | ||
| return function (target) { | ||
| return function (...args) { | ||
| warnExperimental(p5, getPInst(this), subjectArea); | ||
| return target.apply(this, args); | ||
| } | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ import { Image } from '../image/p5.Image'; | |
| import { Texture } from '../webgl/p5.Texture'; | ||
| import { makeFilterShader } from '../core/filterShaders'; | ||
| import { getStrokeDefs } from '../webgl/enums'; | ||
| import { markExperimental } from '../core/experimental'; | ||
|
|
||
| const { STROKE_CAP_ENUM, STROKE_JOIN_ENUM } = getStrokeDefs(() => ''); | ||
|
|
||
|
|
@@ -2356,6 +2357,7 @@ function renderer3D(p5, fn) { | |
| } | ||
| return this._renderer.createStorage(dataOrCount); | ||
| }; | ||
| p5.registerDecorator('p5.prototype.createStorage', markExperimental('webgpu', p5)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Compute shaders only actually work in WebGPU mode and will throw an error in WebGL mode so I figured that path was less critical to flag. |
||
|
|
||
| /** | ||
| * Returns the default shader used for compute operations. | ||
|
|
@@ -2551,6 +2553,7 @@ function renderer3D(p5, fn) { | |
| } | ||
| return this.baseComputeShader().modify(cb, context, { hook: 'iteration' }); | ||
| }; | ||
| p5.registerDecorator('p5.prototype.buildComputeShader', markExperimental('webgpu', p5)); | ||
|
|
||
| /** | ||
| * Dispatches a compute shader to run on the GPU. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ import { Renderer3D } from '../core/p5.Renderer3D'; | |
| import { Shader } from './p5.Shader'; | ||
| import { request } from '../io/files'; | ||
| import { Color } from '../color/p5.Color'; | ||
| import { markExperimental } from '../core/experimental'; | ||
|
|
||
| async function urlToStrandsCallback(url) { | ||
| const src = await fetch(url).then(res => res.text()); | ||
|
|
@@ -746,6 +747,7 @@ function material(p5, fn) { | |
| fn.buildFilterShader = function (callback, scope) { | ||
| return this.baseFilterShader().modify(callback, scope); | ||
| }; | ||
| p5.registerDecorator('p5.prototype.buildFilterShader', markExperimental('p5.strands', p5)); | ||
|
|
||
| /** | ||
| * Creates a <a href="#/p5.Shader">p5.Shader</a> object to be used with the | ||
|
|
@@ -1574,6 +1576,7 @@ function material(p5, fn) { | |
| fn.buildMaterialShader = function (cb, scope) { | ||
| return this.baseMaterialShader().modify(cb, scope); | ||
| }; | ||
| p5.registerDecorator('p5.prototype.buildMaterialShader', markExperimental('p5.strands', p5)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep, good catch! |
||
|
|
||
| /** | ||
| * Loads a new shader from a file that can change how fills are drawn. Pass the resulting | ||
|
|
@@ -1792,6 +1795,7 @@ function material(p5, fn) { | |
| fn.buildNormalShader = function (cb, scope) { | ||
| return this.baseNormalShader().modify(cb, scope); | ||
| }; | ||
| p5.registerDecorator('p5.prototype.buildNormalShader', markExperimental('p5.strands', p5)); | ||
|
|
||
| /** | ||
| * Loads a new shader from a file that can change how fills are drawn, based on the material used | ||
|
|
@@ -1956,6 +1960,7 @@ function material(p5, fn) { | |
| fn.buildColorShader = function (cb, scope) { | ||
| return this.baseColorShader().modify(cb, scope); | ||
| }; | ||
| p5.registerDecorator('p5.prototype.buildColorShader', markExperimental('p5.strands', p5)); | ||
|
|
||
| /** | ||
| * Loads a new shader from a file that can change how fills are drawn, based on the material used | ||
|
|
@@ -2213,6 +2218,7 @@ function material(p5, fn) { | |
| fn.buildStrokeShader = function (cb, scope) { | ||
| return this.baseStrokeShader().modify(cb, scope); | ||
| }; | ||
| p5.registerDecorator('p5.prototype.buildStrokeShader', markExperimental('p5.strands', p5)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a trick that you may or may not want to use (I only just thought of it so it may not work) which is that the pattern matcher (first argument) can be a function and if it returns true the decorator will apply and false it won't, which means that if say you add some flags onto the functions itself to mark it as experimental: p5.prototype.buildStrokeShader.experimental = trueyou can just call |
||
|
|
||
| /** | ||
| * Loads a new shader from a file that can change how strokes are drawn. Pass the resulting | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"WEBGPU mode is experimental, and API related to it (functions and constraints) may change in future versions. You can get involved by giving feedback to help direct its development!"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do constraints refer to in this context?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah sorry "constants"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be rephrased I am just trying to be explicit that API may change (and that means functions or constants may change) in the future