[Bf-animsys] Fwd: GP v2 - Code Review

[Joshua Leung]

Let's move future discussions about GP dev to the bf-animsys list :)

As I proposed during today's meeting, I'd like to propose that we split the
changes into 3-sets:
   1) simple stuff that can go in regardless of the other changes  (e.g.
the stuff under #2 - there are a few other things introduced recently that
also fit in here too),
   2) the layers vs colors stuff -  this is the main bulk of the patch, and
will need the most careful checking, just to make sure that we're all happy
with it (since it will be quite a large change),
   3) stuff which touches other parts of Blender and needs further
discussion with the other module teams (e.g. compositor integration)


Cheers,
Joshua


---------- Forwarded message ----------
From: Joshua Leung <aligorith at gmail.com>
Date: Mon, Jul 18, 2016 at 2:28 AM
Subject: GP v2 - Code Review
To: antonioya blend <blendergit at gmail.com>
Cc: Matias Mendiola <info at mendiobox.com>, Daniel Martínez Lara <
web at pepeland.com>


Hi Antonio,

I thought it would be good to go through the changes that have been
happening in the GP v2 branch so far, to make sure that everything is going
on track for inclusion in master later down the track.

So, here are some comments from what I've seen so far...

Cheers,
Joshua

=============================================

*1)* *"continuous drawing is enabled using double D key"*
https://git.blender.org/gitweb/gitweb.cgi/blender-staging.git/commit/d619edf64c24a19110ee190e911de360fb9b0dea

Interesting idea, and I hope that it works out in testing. However, when
I've tried these kinds of key mappings in the past, I've found that it's a
very bad idea to try mixing "hold-drag" hotkeys (e.g. D-LMBdrag) vs
double-press (e.g. D-D).

The issue mainly comes about if you usually keep the DKEY held down,
release a little bit after doing a normal stroke, then, *very quickly*
press DKEY again and try to draw a second stroke ==> What's likely to
happen is that before you manage to get the pen back down for the second
stroke, the double-press handling has already fired, AND it is likely that
it might've also started drawing straight away (so the line will have low
pressure, be in a bad position, and also be slightly wonky for a bit). What
ends up happening is that it start feeling quite flaky.

Anyway, just mentioning this so that you guys are aware of it and can test
it out further.

[EDIT/CORRECTION:  I just checked the code for this. My only concern is
that this is probably going to just push the event handling for Grease
Pencil drawing past the point where it gets unmanageable. To be fair, it's
actually quite bad already :)  But I think that it may be a good idea if we
first did some cleanup here in master (i.e. separately from all the
exciting new stuff going on here in this branch), to lay the foundations
for what's coming up next).]



*2) There are quite a few tweaks which I'd be happy to include in master
now (if Daniel and Matias are satisfied that they work well).* From the
looks of things, these are general usability tweaks that don't really
depend on the big features anyway, so we can start with these first since
it's easier.

Clear candidates for inclusion:
    - "*Use ctrl/alt for straight line drawing*"  - Sounds like a good
idea. I'll test it a bit and to check how well it works
    - "*Invert F/Ctrl-F*"  -  Let me know how well this works. IIRC, this
is one of the things that I'm not really that happy with currently, so if
it all works well, that'll be great.


Candidates which might need a little closer review first:
   - *Joining strokes together*  - This is something that would be great to
have. It's great to see that you've implemented it already, so I won't have
to, haha :D

   - *"Ctrl-H  for Hiding Points"* - My big question would have to be - how
well is this working for you guys (Daniel/Matias?).
      (On a side note, it's better to just do   "ToolSettings *ts =
CTX_data_tool_settings(C);"  instead of accessing via Scene, if you
don't/won't need access to anything in scene. Also, if you can get scene
(which is almost always guaranteed), then toolsettings will never be null.
And IIRC, it might be better to use one of the poll callbacks for editable
strokes instead of the "editmode_toggle" one)


*3) Grease Pencil and Compositor*
While it's nice to see this functionality, I strongly suggest that you run
double-check with the other key devs who deal with this stuff (in
particular Sergey, but probably also Campbell, Severin, and Ton) that the
approach taken aligns with future development plans for both the layers
system, but also the viewport project and/or Blender Internal modernisation
projects. For instance, I remember Ton saying that he was keen for there to
be a more general "OpenGL pass" in the compositor instead of just a "Grease
Pencil" specific solution.

IIRC, last time this came up, it just so happened that most of the key
people here were also away on holiday that week (or weeks).


*4) Brush Presets*
Looking at this a bit, I suspect that it might be better to look into
defining a set of these in Python scripts (like the other presets)?


*5) Object/Bone Parenting*
First, it's great that we've finally got this in place :D

The main thing I think we need to work on here is more about how this
integrates into Blender - specifically, how this fits with the normal
rigging pipeline and/or integrates with other objects. I think I mentioned
this last time already, but IMO "layer transformed by object" should by
default happen if the GP datablock is attached to an object. (Maybe we can
still keep the current object/bone properties for usage with scene-linked
GP datablocks?)

That way, it fits into the normal object-handling system of Blender more.
Once you do that, you could have it so that hiding the object shows/hides
the GP stuff associated with it, and later on, the restrict render option
could be used to control whether the GP strokes get included in the render
(via autoconverting the GP strokes to curves during the "Blender data to
renderdata conversion" - e.g. 'convertblender.c' or the equivalent for
cycles)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.blender.org/pipermail/bf-animsys/attachments/20160718/600533c8/attachment.html>