Skip to content

SeExpr: structured testing fixes.

This MR fixes various issues detected with SeExpr in Krita Artists.

Bug list from structured testing

  • If the name is the same as the existing resource, the resource folder will not generate or overwrite the old file 🟥
  • In addition, there is no uniform specification for file names in resource folders. Sometimes it is “123.KSE”. Sometimes it is “seexpr_scripts123.kse” 🟥
  • When you click “overwrite preset”, this button does not turn gray, and the “reload the preset” button always exists. If you click “reload the preset” at this time, krita crashes 🟥
  • You cannot change the name, a new file will appear in the resource folder, it will still have the same name as before 🚗

The following bugs are strictly resource system purview (depends on !1072 (merged)):

  • Cannot overwrite existing resources when importing 🟥.
  • After deleting a resource, creating another resource with the same name is invalid 🟥
  • After deleting a file, importing a file with the same name will have the same problem as above, and the resource will not appear
    • Tiar says these two are one and the same. Halla says we should implement a "preset reuse" dialog, which is outside the scope of this resource type
  • Sometimes the resource cannot be rendered. :red_square: The resources in the resources folder have not changed 🟥

This MR completes changes made in 126565fc.

Bug list from feedback and improvements thread

  • Rename feature
    • included above
  • When saving preset with empty thumbnail krita crashes.
  • There is no clear indication that the script is stored to a file
    • This relies on the "dirty" indicator, which is fixed above. Overwrite Preset is !1072 (merged) scope.
  • Unable to change icon for the script postfactum
  • Stability.
    • This is triggered by a runtime assertion on usage of curve and ccurve with invalid interpolation parameters.
  • Unclear checkbox
    • This is used upstream, but I can try hiding them.
  • Script edit
    • The tab symbol is too large. I would set it to default of value 4 (and replace with spaces).
      • (making it configurable would require discussion and would be for 5.1)
    • Line numbers. For better error messages interpretation and debugging adding line numbers to editor would be nice to have.
      • Subsumes "There is no way for a user to understand what 76 and 77 mean." (Qt cursors think as indexes on a string instead of line/columns.)
    • Differentiate between different assignments of the same variable
      • For a future release, this needs parser support.
    • Signal to the user that the script is being rendered
      • (Halla:) That would be good, but an hourglass during the editor freeze would be good to have – if we don’t have that already.
      • EDIT (Oct 5): we already do, the preview generator gets an instant snapshot of the script code and the layer docker shows the progress.
  • 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.
    • Out of scope; although the resource system now has overrides for bundled presets.
  • 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.
    • Out of scope

Bug list from b.k.o

BUG: 442853

Test Plan

Build Krita and structure test SeExpr scripts.

Formalities Checklist

  • I confirmed this builds.
  • I confirmed Krita ran and the relevant functions work.
  • I tested the relevant unit tests and can confirm they are not broken. (If not possible, don't hesitate to ask for help!)
  • I made sure my commits build individually and have good descriptions as per KDE guidelines.
  • I made sure my code conforms to the standards set in the HACKING file.
  • I can confirm the code is licensed and attributed appropriately, and that unattributed code is mine, as per KDE Licensing Policy.
Edited by Amy spark

Merge request reports

Loading