Feedback on Patch Templates

Feature & Design Feedback
1

A while ago I though it might be useful to have some example patches so people who weren’t sure where to start see modifications needed to make changes to Blender.

I added example modifiers (deform & constructive), with the intention of nodes, space types buttons etc.

See https://github.com/ideasman42/blender_patch_templates which has example patches and gives a detailed rationale, pros and cons.

Maintaining patches in a repository isn’t so nice, it would be better to have these as branches.

The main down-side for this is the branches would need to be occasionally need to be updated to apply to master.

I’m curious if…

  • Anyone is interested to update the patches create new patch templates, generally help out.
  • New developers would find this useful.

On the other hand, I’m aware there are arguments against doing this too:

  • Enables cargo-cult programming.
  • Figuring this out for yourself may be a better way to really learn.
  • New developers can always read commit logs for similar additions).
5 Likes
2

I definitely find this useful. Sure, it’s possible to figure out for yourself, but having a starting reference will remove the initial friction, teach common workflows and idioms and ensures you don’t miss anything important.
Actual commits usually have too much non related, or even misleading info.

However, a few observations:

  • Unless these templates (and non optional unit tests) get into the main repo, they will become outdated sooner or later. The wiki is a good example of this.
  • I would appreciate more in depth info about the big picture and the non obvious details. Things that are harder to figure out by just reading the code like “What will happen to my files if I change the RNA declarations or I save them with other branch?”,“Is there anything I shouldn’t do from here? Should I take concurrency into account?” or “What should I take into account when making conversions between different types of meshes?”.

Maybe a complementary video for each template could fill the gaps ? Or even just live-stream the templates development ?

3

Video’s are far too time-consuming to update.

If information is related to the patch-template, these can just be comments in the patch.

As for bigger picture docs, I still rather keep this part of comments, or perhaps the wiki in some cases.

Examples of more comprehensive docs in code (bmesh.h, WM_types.h, DNA_meshdata_types.h).

4

I was proposing live-streams precisely because I find them less time consuming to produce. In one hour of rambling you can provide way more info than if your try to come up with a well structured written equivalent in the same time.

But I actually prefer written docs. Complete and commented examples are much better, of course.

The problem with the docs in the code is that they aren’t always there, but maybe it’s better to just change that instead?

5

The problem with docs in code is that they can be hard to find, most people starting out won’t even know they are there. Having the generated doxygen html online and linking to them from the index would make it easier IMO.

As for patch templates, why not add a snippets section/webpage where we list common things like adding a property or op, creating a macro, accessing context, etc. I remember seeing a gist sometime ago for bpy stuff, it’s actually pretty useful.

6

I was initially a bit skeptical about whether we should be providing such templates. In particular, two problems come to mind:

  1. There’s the problem of keeping these up to date with changes in the main branches - A potential solution could be to have a repo somewhere (or maybe as part of the main repo, but we’d have to disable a bunch of annoying “features” that have been enabled) where each template has its own branch (with a prefix like devtemplate-, containing just a single stub commit. We could have it so that once a month (?), these templates get rebased off the master/2.8 branch - probably through some automated script like the existing buildbots; if merge conflicts arise, the branches in question would get flagged for human intervention.

  2. Such templates cannot really help when you’re trying to add stuff that needs to be defined on a particular operator/keymap-thing (e.g. adding another type of builtin keymap like ED_GPENCIL / ED_MARKERS, or adding circle select modal handling capabilities to an operator) that may occur in any arbitrary file. It’s fine however if you’re just adding a new ID type, modifier/constraint/etc. though. Other approahces though can handle such cases better though (more on this in a bit).
    (EDIT: Maybe the .rst files help resolve the problem of noting things that cannot be encoded in a patch, but it feels like a bit of extra overhead - i.e. having to now maintain two artifacts in addition to the “live” code repo)

In defense of this approach though, it occurred to me looking at these that by having patch templates, you’re providing devs with a “stub commit”, making it easier to see (via the Git staging area) which parts of the patch they’ve sorted out, and which still need to be done. If you set up your workflow so that you stage each hunk as you’ve gone through customising the code, this could turn out to be quite a useful self-check/progress indicator thing.

I should also note that a while back, I also started a little initiative to try documenting some of these. Sadly I haven’t had time to fill out many of the others yet, but it’s probably worth mentioning this here too:
https://wiki.blender.org/index.php/Dev:Source/Architecture#Developer_Checklists

1 Like
7

The problem with video (and audio) based material is that it’s a bit of a pain to seek/extract information out when you actually come to doing it. It’s good for “big picture” explanations, and/or sometimes as a useful aid to supplement a written guide (e.g. sometimes, some procedures when written down look really “scary” and “complicated”, while in practice, if you see someone doing it, it turns out to be a simple split-second thing that can happen quite naturally), but I wouldn’t want or recommend using it the primary or sole method of communicating technical info like this.

8

Not sure what you mean by:

As for…

Agree having patch-template-* branches would be the way to go, was going to write more on this, along the lines of what you’ve written.
At the end of the day if this doesn’t work out - we can simply delete all the branches too.

Right, I don’t think we would attempt some kind of full coverage. On the other hand - for someone starting out - making any kind of change to Blender can be overwhelming, so a way to look at an existing patch could be a good way to get a feel for the code in an area you’d like to start developing. Even if it’s not helping you make the exact change you’re looking into.

One advantage of maintaining a branch is you get a hard failure when there is a conflict, unlike a tutorial maintained separately and nearly always get outdated.

Another advantage of having patch-templates is experienced devs can use them (I used them for new modifiers - just as a quick way to avoid manually adding a bunch of boiler-plate), although this isn’t justification for having them in the first place - it’s a minor up-side.

The main down-side to doing all of this is time time it takes to create and update the branches. So questions are:

  • Should we go ahead with creating the branches with some updated templates?
  • After enough time (eg: ~6-12months) we can re-evaluate if this is helping, or more trouble than it’s worth.

Is anyone against trying this out?

9

I expect I’d find this very useful but don’t have any skin on the game. I’d like to see it happen if it’s jot too much work for the devs building/maintaining them

10

@ideasman42 Oh, I was referring to how IIRC “git push -f” is blocked, so we can’t just replace the branches that easily (well, you could, but it would mean deleting the branch off the main repo first).

11

Delete and re-create is fine AFAICS, people wont be keeping long-running updates of these branches.

12

I see more value in documenting the steps needed to perform a certain task than to just throw a patch at someone. this guide by troy_s was more useful to me when i was adding a compositor node, than just throwing a patch at me that would have done the same work. It’s a little sparse on the actual documentation, but at least it lists the steps and gives somewhat of a motivation ‘why’ we are taking the step.

however, as educational as this easy 17 step process was to add a compositor node, it’s still super time consuming and most of it seemed boilerplate to me, so i automated the work with a little helper script.

AddCompositorNode.py

I’m not gonna lie, this is probably my second python script, it’ll probably violate everything in pep8, but it does what i need it to do.

Still i see more value in offering and maintaining a step by step guide for common tasks (with convenience scripts like these to automate the work) (not volunteering here, i just see more value in it ) than just throwing a patch at someone and go ‘here’s everything all together, figure it out’

13

I’ll push back a that usin a patch means not documenting steps, a good patch-template would have comments explaining whats going on too.

Saying “here’s everything all together, figure it out” is what developers already do all the time when reading commit logs and doing code-review. This is a useful skill and something developers should learn sooner than later. As stated already, if the patch is well commented I think it has the benefits of a tutorial with the advantage it can be automatically validated to apply correctly.

14

I think that expecting a new developer that has no prior exposure to our code base will be able to extract this ‘easy 17 step plan’ from a single patch by tossing in a comment or two might be a little on the optimistic side of things, and personally think we need to offer more guidance on how to do common basic tasks.

1 Like
15

I think this is a pretty good idea. I basically saved our a stub branch for adding a modifier when I did the corrective smoothing stuff.

Even if they are not 100% up to date, so long as they show what areas of the codebase are involved it makes it much easier to get started; I always find discovery one of the harder parts of jumping into a new codebase. If you know roughly where to look it is much easier to figure out.

I would certainly prefer this to any kind of video content, which I can’t stand for dev instructions/tutorials.

16

Existing developers started largely without much in the way of any kind of dev tutorials, it’s not that I’m optimistic, its that this may be a lot better than nothing if someone is trying to get their feet wet in an area of code they’re unfamiliar with.

There are pros and cons, on the flip side - why spend some hours following a tutorial to discover it’s out-dated and doesn’t even compile once you’re done (which is often the case for dev docs that don’t get updated).
At least with a branch you can start from a working example, if it’s broken you know early on and thorough no fault of your own.

At the end of the day this is about whats useful, and what developers are happy to maintain. Personally I’d prefer keep a branch updated than a tutorial.
To check if a tutorial works - I need to manually follow it each time (which ends up not getting done)… or wait for someone to complain it doesn’t work. Checking a branch still works is much more straightforward.

17

While that may work for some developers, it’s hard to deny there’s a hand full of you guys at best, hell we started this site to ease new devs into the blender codebase.

That being said, I’ll take a well kept branch/patch set over a neglected tutorial anyday (troys tutorial was outdated,I followed it and fixed the bits that were broken) so if patches are the way you want to go, i can only encourage it.

18

Hi,

I think @ideasman42 's initiative about patch templates can be the good place to provide a feedback from a newbie to Blender development (as I am), even if all below is not only related to patch templates.

Few words about me (you probably don’t care, but that can explain some of the “why” about the points I will mention further). So, I’m 52, from France, and I love my wife and my cats.
Concerning coding: before 1990 (personally or as student) C, Pascal, Fortran, Lisp, 1990-2002, C++, from 2002 C# (and a bit of Javascript, Python). So my C/C++ knowledge is now far away.
Concerning Blender: as hobbyist from 2006 or around, from 2010 also using it for my work (illustrations for presale actions). Mainly modeling and shading, coding few little addons or operators. Sometimes active participant in BSE.

That said, I’ve decided recently (around 2 weeks ago) to go further trying to participate to Blender development. Here is my feedback about it (please consider nothing here is for criticizing, I’m just wondering about many points):

  • My primary aim: coding a “Python modifier”, a modifier that accepts Python addons.
  • To learn to do that, I’ve decided to first make few simple modifiers (or I thought it was simple)
  • The first one (done): a “randomize modifier” (randoms mesh vertices from some parameters: axis, other object axis, vertex group)
  • The second one (failed so far): a “remove doubles modifier”. I’ll explain why I failed below.
  • The third one (planed): a “fan modifier” which is some kind of mix between screw and array and will be a port of a Python operator I’ve coded before.

The “remove doubles modifier”: I thought it was simple as the function already exists somewhere from an operator. But diving into the code, I’ve understood (may be wrong):

  • It is operators data dependent for some parts
  • It relies on kdtrees
  • It seems that the function cannot be directly reused in a modifier context

So my thought was the following:

  • The remove doubles algorithm should be isolated in another library
  • By the way, this lib could be C++ based (except from extern parts), as I’ve seen some parts of Blender are coded using C++
  • C++ can provide the very versatile data structures of std libs as I believe this kind of structures is a key element to code algorithms (the same way lists, arrays, dictionaries and sets are key points for Python) and Blender contains plenty of algorithms
  • But… how to add a new library to Blender? I’ve spent a day on it and still don’t know how to add one (where is the how to for that?). I think a “patch template” can be usefull for that (or simply a documentation, but I was not able to find one).

More general aspects:

  • Coding a feature is sparsed in several levels
  • In my opinion, this is the why cookbooks are required (or patch templates) if you want to do anything. And that means that anyone who want to do anything needs to modify code parts that is not related to what he/she wants to do (example: rna_modifier.c contains definitions for all modifiers)
  • I know we need to make the bridge between a feature (say a modifier) and Python GUI through DNA and RNA, but (again in my newbie opinion) this is not good to sparse that at different levels of the architecture
  • I think, ideally, each complete feature (eg a modifier) should correspond to one set (a library) isolated from the other features (eg the other modifiers), having in its own scope all the needed parts (eg the modifier itself and its DNA and RNA parts)
  • These independent features/modules should themselves be internally architectured in respect of the said levels/aspects through one or several “interfaces” (in the design pattern meaning).
  • First thought, that probably could be done extending ModifierTypeInfo and/or ModifierData
  • Pros: more simple to patch, maintain, explain, less impact on any other Blender’s architecture parts, allows to choose between C and C++ for coding, could open the way for C/C++ libraries plugins/addons
  • Cons: maybe there is, please tell

Is there a way to do that progressively (for instance, if we want to do that on modifiers, is it possible to do it without ‘migrating’ all the modifiers in one step)? I think yes (but again I’m a newbie), and of course if I wanted to try a proof of concept about it, well… knowing how to add a new lib (as said above) is a key…

At this step, I’m wondering if I’m wrong or right writing all that as I know few and have done nothing so far. In some aspects, this is comparable to be a Blender user: this is not simple at the beginning but once the principles are acquired, you feel it great and it is a pleasure to use it… I hope I’ll feel the same coding for it, with time…

… ok, its time to click on the “reply” button.

Thanks for reading

19

As very fresh , like one day old, Blender developer. I don’t see the point of these templates.

The way I understand after reading the relevant documentation at Blender wiki is that a patch could be created by selecting my commits using tortoisegit and instructing it to create a patch that I can submit with my modifications/additions together with a readme file for documentation and maybe a video tutorial if its really needed.

Why would I need a template to do that ?

Adding functionality to Blender is a complex process that needs a deep understanding of the area of the source one works on. This can happens either by documentation or when documentation does not exist , by reading the code, creating a debug build, launching the build with a debugger and use the debugger to examine the structure of code execution and the live data and how it changes overtime.

At least this is how I do it and the workflow I use to learn Blender internals.

Reading code via patches is definitely not the way I would like to do it and I don’t see the advantage.

If the focus is to teach how Blender code works, for me that can happen either by video tutorials, diagrams/illustrations or regular documentation.

20

Hey Campbell, it’s me J_B and I managed to push the “template-object-add-mesh” branch to the Blender Staging repository. I don’t know if I did anything wrong or not. I tried following the instructions :

git remote add origin_staging [email protected]:blender-staging.git
git push --set-upstream origin_staging my_feature_branch

There is a couple of additions that I didn’t mean to get included, and actually I don’t even yet know how not to include them.

release/scripts/addons_contrib
release/scripts/addons

Also, I forgot to note that I didn’t include the uv unwrapping code originally started in commit “3c6709a63” because I don’t understand that code. I probably should have included a broader description of what the code does too.

If I did anything wrong let me know.