SeExpr Feedback and improvements request

[yetanotherpainter]

SeExpr feedback

First of all I want to say that I am impressed by what is already could be achieved with SeExpr option in Fill Layer. I think developers did a good job!

While playing around with this option I gathered some comments and developers are free to pay attantion to them, or not

Here we go.

1: Rename feature
It seems that a pencil next to the script name does not rename the script at all.

2: When saving preset with empty thumbnail krita crashes.

Krita Crashes while saving SeExpr script with empty thumbnail

You missed a step: 3a. On the Scripts tab, select one of the thumbnail images. I see this with the Sept 02 5.1.0-prealpha appimage. Terminal output: kf5.ki18n: Trying to convert empty KLocalizedString to QString. kf5.ki18n: Trying to convert empty KLocalizedString to QString. Dialog “krita/generators/seexpr” has a widget without an object name: QScrollBar(0x30a1bb10) Dialog “krita/generators/seexpr” has a widget without an object name: QScrollBar(0x309e33f0) Segmentation fault Looks lik…

3: There is no clear indication that the script is stored to a file

No icon that script is unsaved (like asterisk on a filename for instance)
Pressing ‘Overwrite Preset’ by the look of it changes nothing.

The script is seems to be saving, but you are not sure.

Actually that full unclear saving workflow lead me to accidentally replace one of the presets from Krita.

4: Unable to change icon for the script postfactum

There is no way to update the icon for a preset. I guess you have to make a copy and add the icon, then remove the original.

Would be nice to have an ability to edit the preset to change icon.

5: Stability.

Krita crashes from time to time while doing different actions with the script.
Would be nice to get it a little bit more stable.

6: Unclear checkbox

Each variable has a checkbox. Some of the checkboxes can be check, some of them cannot.
Either way it is not clear what these checkboxes do.

7: Script edit

7a: The tab symbol is too large. I would set it to default of value 4 (and replace with spaces). As it is the most common format. (I know, I know this can be a curly braces arguement, but still).
Maybe let the user configure it.

7b: Error messages would be good to improve. For instance for this piece of code:

if ($u > 0.5) {
    $color = [0.5,0.5,1];
}
else {
  $color = [1, 0.5, 0.5]
}

$color

The error message states:
(76, 77): Syntax error near ‘}’

There is no way for a user to understand what 76 and 77 mean. And that brings next point.

7c: Line numbers. For better error messages interpretation and debugging adding line numbers to editor would be nice to have.

7d: Not sure if it at all possible, so this one might be ignored. Though.
By having condition like this:

if ($u > 0.5) {
    $color = [0.5,0.5,1];
}
else {
  $color = [1, 0.5, 0.5];
}

Editor will create two variables with the same name ‘color’. So for the user (who will not even look at code) it would be beneficial to see some meaningful name.
Maybe a way to add alias to the variable names when they displayed on the middle panel.

7e: Once I finish editng (looks like a compilation happening) the editor freezes for a bit (that might mislead the user that the application just stuck and will not work anymore). Would be good to have some indicator for user to understand that he has to wait wile it finished.

Another thing to look at is to give the user the control when to run compilation. Krita might work/compile in two modes: automatic (tries to compile when it thinks more appropriate), manual (user clicks on the button, or presses a shortcut)

8: Lock editing for scripts shipped with Krita. It is easy to mess things up and loose scripts shipped with krita. Maybe making such that the user have to make a copy first, and then edit it.

9: This is one is more like an idea. But… Node aditor for SeExpr. Something similar how Blenders’ Node based material editor with Node Wrangler works. Would be a killer feature.

Cheers!

P.S. I am adding system info so the developers can at least have a point of reference.

Summary

Krita

Version: 5.0.0-beta1 (git 2e95dd5)
Languages: en_US, en
Hidpi: true

Qt

Version (compiled): 5.12.11
Version (loaded): 5.12.11

OS Information

Build ABI: x86_64-little_endian-llp64
Build CPU: x86_64
CPU: x86_64
Kernel Type: winnt
Kernel Version: 10.0.19043
Pretty Productname: Windows 10 (10.0)
Product Type: windows
Product Version: 10

OpenGL Info

Vendor: “Google Inc.”
Renderer: “ANGLE (NVIDIA GeForce GTX 1080 Ti Direct3D11 vs_5_0 ps_5_0)”
Version: “OpenGL ES 3.0 (ANGLE 2.1.0.57ea533f79a7)”
Shading language: “OpenGL ES GLSL ES 3.00 (ANGLE 2.1.0.57ea533f79a7)”
Requested format: QSurfaceFormat(version 3.0, options QFlagsQSurfaceFormat::FormatOption(DeprecatedFunctions), depthBufferSize 24, redBufferSize 8, greenBufferSize 8, blueBufferSize 8, alphaBufferSize 8, stencilBufferSize 8, samples -1, swapBehavior QSurfaceFormat::DoubleBuffer, swapInterval 0, colorSpace QSurfaceFormat::DefaultColorSpace, profile QSurfaceFormat::CompatibilityProfile)
Current format: QSurfaceFormat(version 3.0, options QFlagsQSurfaceFormat::FormatOption(), depthBufferSize 24, redBufferSize 8, greenBufferSize 8, blueBufferSize 8, alphaBufferSize 8, stencilBufferSize 8, samples 0, swapBehavior QSurfaceFormat::DefaultSwapBehavior, swapInterval 0, colorSpace QSurfaceFormat::DefaultColorSpace, profile QSurfaceFormat::NoProfile)
Version: 3.0
Supports deprecated functions false
is OpenGL ES: true

QPA OpenGL Detection Info
supportsDesktopGL: true
supportsAngleD3D11: true
isQtPreferAngle: true

Hardware Information

GPU Acceleration: auto
Memory: 65470 Mb
Number of Cores: 6
Swap Location: C:/Users/rapid/AppData/Local/Temp

Current Settings

Current Swap Location: C:/Users/rapid/AppData/Local/Temp
Current Swap Location writable: true
Undo Enabled: true
Undo Stack Limit: 30
Use OpenGL: true
Use OpenGL Texture Buffer: true
Disable Vector Optimizations: false
Disable AVX Optimizations: false
Canvas State: OPENGL_SUCCESS
Autosave Interval: 900
Use Backup Files: true
Number of Backups Kept: 1
Backup File Suffix: ~
Backup Location: Same Folder as the File
Backup Location writable: false
Use Win8 Pointer Input: false
Use RightMiddleTabletButton Workaround: false
Levels of Detail Enabled: false
Use Zip64: false

Display Information
Number of screens: 2
Screen: 0
Name: \.\DISPLAY1
Depth: 32
Scale: 1
Resolution in pixels: 2560x1440
Manufacturer:
Model:
Refresh Rate: 59
Screen: 1
Name: \.\DISPLAY2
Depth: 32
Scale: 1
Resolution in pixels: 2560x1440
Manufacturer:
Model:
Refresh Rate: 59

[dkazakov]

I think we should summon @amyspark here

[amyspark]

I’m the maintainer of the SeExpr fork for Krita. I’ll review your points below:

@yetanotherpainter

1: Rename feature
It seems that a pencil next to the script name does not rename the script at all.

Tested it in commit 6ca8117f88, the rename is applied against our resource database. The bundle itself is not touched at all. (cc @halla, is that the correct behaviour?)

@yetanotherpainter

2: When saving preset with empty thumbnail krita crashes.

Confirmed. Filed bug 442096. I need to ask tomorrow if we should set at least a blank thumbnail by default.

@yetanotherpainter

3: There is no clear indication that the script is stored to a file

I don’t know if we have this behaviour for other types of resources, I’ll ask at tomorrow’s meeting and reply back. (In any case, as soon as the dialog is closed, the layer will make an independent copy of the script, so it’ll render the overwrite pointless.)

@yetanotherpainter

4: Unable to change icon for the script postfactum

Same as 3 – SeExpr scripts are immutable once saved (except for renames, for the reason above), although it’d be definitely a good thing to have.

@yetanotherpainter

5: Stability.

This is the trickiest bit of all. Disney’s way of coding SeExpr means that there are over 100+ asserts throughout the code; the only way to fix this is with a lengthy refactor. I’ll ask this too at the next meeting.

@yetanotherpainter

6: Unclear checkbox

The checkboxes signal events that are supposed to be consumed by an (unknown) Disney tool, so I am afraid I cannot remove them. (Probably some node wrangling tool?)

@yetanotherpainter

7a: The tab symbol is too large. I would set it to default of value 4 (and replace with spaces). As it is the most common format. (I know, I know this can be a curly braces arguement, but still).
Maybe let the user configure it.

Qt thinks the tab size is 80 pixels (!!!). Perhaps I could lower it in code, we’ll review it at the next meeting.

@yetanotherpainter

7b: Error messages would be good to improve. For instance for this piece of code:

@yetanotherpainter

7c: Line numbers. For better error messages interpretation and debugging adding line numbers to editor would be nice to have.

Those two are related; I don’t know if it’s possible to implement them, given the way Disney implemented its language parser. I’d have to check during the refactor.

@yetanotherpainter

7d: Not sure if it at all possible, so this one might be ignored. Though.
By having condition like this:

if ($u > 0.5) {
    $color = [0.5,0.5,1];
}
else {
  $color = [1, 0.5, 0.5];
}

Editor will create two variables with the same name ‘color’. So for the user (who will not even look at code) it would be beneficial to see some meaningful name.
Maybe a way to add alias to the variable names when they displayed on the middle panel.

This is not possible to do (afaik), since to the parser those are two different assignments, and hence they generate separate widgets. Do you feel a better alternative would be to (in case of aliasing such as this) disambiguate by position, if available?

@yetanotherpainter

7e: Once I finish editng (looks like a compilation happening) the editor freezes for a bit (that might mislead the user that the application just stuck and will not work anymore). Would be good to have some indicator for user to understand that he has to wait wile it finished.

Yes, some hundred ms after the last keystroke event Krita triggers a repaint; as part of that, and to ensure nothing changes the script’s contents, I temporarily block the editor. We could add an automatic/manual preview toggle, what do you think @halla?

@yetanotherpainter

8: Lock editing for scripts shipped with Krita. It is easy to mess things up and loose scripts shipped with krita. Maybe making such that the user have to make a copy first, and then edit it.

Does such a feature exist in our resource system @halla?

@yetanotherpainter

9: This is one is more like an idea. But… Node aditor for SeExpr. Something similar how Blenders’ Node based material editor with Node Wrangler works. Would be a killer feature.

Definitely nice to have, but would involve so much effort it’d be a GSoC on its own!

[yetanotherpainter]

@amyspark

I don’t know if we have this behaviour for other types of resources, I’ll ask at tomorrow’s meeting and reply back. (In any case, as soon as the dialog is closed, the layer will make an independent copy of the script, so it’ll render the overwrite pointless.)

Let me clarify. Bullet number 3 is about editing the script. If I want a brand new script I would start with empty document and gradually add the code there. So when I change something (meaning modify the existing script), there is no clear understanding that it is stored and the window can be safely closed.

[yetanotherpainter]

@amyspark

This is the trickiest bit of all. Disney’s way of coding SeExpr means that there are over 100+ asserts throughout the code; the only way to fix this is with a lengthy refactor. I’ll ask this too at the next meeting

Yes. I understand that. And let me be clear. When I request these I ask not demand. I understand that there might be other priorities.
Just for an artist who will work with it, he will get a lot of frustration, what would be preferable to avoid

[yetanotherpainter]

@amyspark

Those two are related; I don’t know if it’s possible to implement them, given the way Disney implemented its language parser. I’d have to check during the refactor.

I assume you have control over the text message itself anyways. So maybe at least remove the numbers to not confuse the user then. Maybe not the prettiest solution, though at least the user will not look at line 30 in his code to fix the issue in line 3

[yetanotherpainter]

@amyspark

This is not possible to do (afaik), since to the parser those are two different assignments, and hence they generate separate widgets. Do you feel a better alternative would be to (in case of aliasing such as this) disambiguate by position, if available?

Hmm. Not sure if I understand correctly “Disambiguate by position”. My point is to make the widget usable such that artist even do not have to look in the code. This is helpful when the code is distributed in a package by someone else.

In this case artist just need to read the docs and go for it.

[yetanotherpainter]

@amyspark Happy to see you working on it
Thanks you.

Btw. In terms of things that are not possible to make due to Disnay’s API. Since the expose it to the world I assume they might accept some requests to improve the API (such as have an ability to hide the checkbox)
As a suggestion.

Looking forward to see SeExpr tool improvements.

[halla]

1: Rename feature
It seems that a pencil next to the script name does not rename the script at all.

Tested it in commit 6ca8117f88, the rename is applied against our resource database. The bundle itself is not touched at all. (cc @halla, is that the correct behaviour?)

There should be new version of the script, with the new name in a folder for updated resources for that bundle.

3: There is no clear indication that the script is stored to a file
I don’t know if we have this behaviour for other types of resources, I’ll ask at tomorrow’s meeting and reply back. (In any case, as soon as the dialog is closed, the layer will make an independent copy of the script, so it’ll render the overwrite pointless.)

Whether resources are visibly handled as files is a bit inconsistent in Krita, with some places looping asking for a filename, and others just doing something under the hood

4: Unable to change icon for the script postfactum
Same as 3 – SeExpr scripts are immutable once saved (except for renames, for the reason above), although it’d be definitely a good thing to have.

5: Stability.
This is the trickiest bit of all. Disney’s way of coding SeExpr means that there are over 100+ asserts throughout the code; the only way to fix this is with a lengthy refactor. I’ll ask this too at the next meeting.

6: Unclear checkbox
The checkboxes signal events that are supposed to be consumed by an (unknown) Disney tool, so I am afraid I cannot remove them. (Probably some node wrangling tool?)

Maybe we can just hide them?

7a: The tab symbol is too large. I would set it to default of value 4 (and replace with spaces). As it is the most common format. (I know, I know this can be a curly braces arguement, but still).
Maybe let the user configure it.

Qt thinks the tab size is 80 pixels (!!!)](QTextEdit Class | Qt Widgets 5.10). Perhaps I could lower it in code, we’ll review it at the next meeting.

Maybe, though 4 spaces sounds like the best option to me.

7b: Error messages would be good to improve. For instance for this piece of code:
7c: Line numbers. For better error messages interpretation and debugging adding line numbers to editor would be nice to have.

Those two are related; I don’t know if it’s possible to implement them, given the way Disney implemented its language parser. I’d have to check during the refactor.

7e: Once I finish editng (looks like a compilation happening) the editor freezes for a bit (that might mislead the user that the application just stuck and will not work anymore). Would be good to have some indicator for user to understand that he has to wait wile it finished.
[/quote]

Yes, some hundred ms after the last keystroke event Krita triggers a repaint; as part of that, and to ensure nothing changes the script’s contents, I temporarily block the editor. We could add an automatic/manual preview toggle, what do you think @halla?

That would be good, but an hourglass during the editor freeze would be good to have – if we don’t have that already.

8: Lock editing for scripts shipped with Krita. It is easy to mess things up and loose scripts shipped with krita. Maybe making such that the user have to make a copy first, and then edit it.
[/quote]
Does such a feature exist in our resource system @halla?

No: the goal of the resource rewrite was to make all default resources editable. In 5.1, we must finish the resource manager dialog, which would allow the user to go back to earlier versions of a resource.

9: This is one is more like an idea. But… Node aditor for SeExpr. Something similar how Blenders’ Node based material editor with Node Wrangler works. Would be a killer feature.

Definitely nice to have, but would involve so much effort it’d be a GSoC on its own!

More than that, with the current cut-down gsocs… If there’s ever going to be another gsoc.

[yetanotherpainter]

@halla

More than that, with the current cut-down gsocs… If there’s ever going to be another gsoc.

It is kinda sad that feature for the software are being developed from one gsoc to another.

I guess it is a nature of open source software.

[tiar]

@amyspark

Tested it in commit 6ca8117f88, the rename is applied against our resource database. The bundle itself is not touched at all. (cc @halla, is that the correct behaviour?)

Yes. It changes the visible name, not the name of the file.

@yetanotherpainter

It is kinda sad that feature for the software are being developed from one gsoc to another.

I guess it is a nature of open source software.

@amyspark mentioned GSOC because the Seexpr was implemented during GSOC. But then they got hired and now they work on stuff outside of GSOC. But there is a bit of truth that features got implemented by GSOC students and volunteers: because features are fun (and somewhat easy - small feature is usually easier than a small bug), and maintaining existing systems and fixing bugs is much more boring, tedious, and… well you kinda need to pay someone to do that. Which is why the paid team does all the boring behind-the-curtain work, and the volunteers get to do the visible stuff.

GSOC students and volunteers are also not restricted by “what Krita team should be doing”. They don’t have the same priorities.

[yetanotherpainter]

I see. In any case, I am happy where Krita team already now.
The tool is already pretty competitive.

I just wish the devs will keep it up
And maybe one day there will be also people in the team who does fun stuff too.

[yetanotherpainter]

To add on things that might be improved with SeExpr editor is this

When you have a code with comment like this for interpreter it should mean that value can be between 1 and 100

$subdivisions = 10; # 1, 100;

However in Krita editor when you start drag slider it goes only between 0 and 1.

[tiar]

It worked in 4.4.1, I just today reported it actually: 442853 – Seexpr widgets no longer respect the range (regression in 4.4.3)

[yetanotherpainter]

I started working with SeExpr more and noticed there is no way to create a user defined function.

It would be nice feature to have, since it will enable user to create his own transformation operators.

EDIT: In that regard. Maybe having separate “Function Preset” mechanics. So that functions can be edited in separate window. This will allow share the functions and manage them with Resource Manager.

[tiar]

If I remember correctly, @amyspark told me it’s only possible if I compile the function into seexpr… which means it’s not something you can use just by writing a script, or having it in GUI

[yetanotherpainter]

Hm. So to have general purposed function one should do a merge request?

EDIT:
It is a bit confusing. When we write the script for pattern it goes the stage of compilation.
Is it some kind of different type of compilation that does not looks for functions and stuff?

The only reason for me can be that particularly for Krita main() function is hidden from the user. So what we can write in UI is inside that function. That might be the reason why we cannot make another functions. And it might explain why we get error message with line number that does not match line number in editor inside Krita.

[yetanotherpainter]

I played with SeExpr a little bit more and here what I have discovered:

Seems like cellnoise functions are not in there anymore:

$nu = cellnoise1($u);
$color= [0,0,0];
$color

Output:
(6, 20): Function ‘cellnoise1’ has no definition
(0, 21): Assgnment operation has incorrect type ‘lifetime_error Error’

Same for cellnoise2 and cellnoise3

---

For ccurve - when we expand gradient - update the result in this window too.

---

Would be nice to have maximum and minumum functions - to know what new range my vectors actually got (to properly remap values)

the interface will be:

float maximum(vector v)
float minimum(vector v)

Version: 5.0.0-beta1 (git 9bd5348)