<div dir="ltr"><div>Let's move future discussions about GP dev to the bf-animsys list :)<br><br></div>As I proposed during today's meeting, <span>I'd like to propose that we split the changes into 3-sets: <br>   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), <br>   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), <br>   3) stuff which touches other parts of Blender and needs further discussion with the other module teams (e.g. compositor integration)</span><div><br><br></div><div>Cheers,<br></div><div>Joshua<br><br></div><div><div><div><br><div><div class="gmail_quote">---------- Forwarded message ----------<br>From: <b class="gmail_sendername">Joshua Leung</b> <span dir="ltr"><<a href="mailto:aligorith@gmail.com">aligorith@gmail.com</a>></span><br>Date: Mon, Jul 18, 2016 at 2:28 AM<br>Subject: GP v2 - Code Review<br>To: antonioya blend <<a href="mailto:blendergit@gmail.com">blendergit@gmail.com</a>><br>Cc: Matias Mendiola <<a href="mailto:info@mendiobox.com">info@mendiobox.com</a>>, Daniel Martínez Lara <<a href="mailto:web@pepeland.com">web@pepeland.com</a>><br><br><br><div dir="ltr"><div><div><div><div><div><div>Hi Antonio,<br><br></div>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.<br><br></div>So, here are some comments from what I've seen so far...<br><br></div>Cheers,<br></div>Joshua<br><br>=============================================<br><br><b>1)</b> <b>"continuous drawing is enabled using double D key"</b><br><a href="https://git.blender.org/gitweb/gitweb.cgi/blender-staging.git/commit/d619edf64c24a19110ee190e911de360fb9b0dea" target="_blank">https://git.blender.org/gitweb/gitweb.cgi/blender-staging.git/commit/d619edf64c24a19110ee190e911de360fb9b0dea</a><br><br></div>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). <br><br>The issue mainly comes about if you usually keep the DKEY held down, release a little bit after doing a normal stroke, then, <i>very quickly</i> 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.<br><br></div><div>Anyway, just mentioning this so that you guys are aware of it and can test it out further.<br><br></div><div>[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).]<br></div><div><br><br><br></div><div><b>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).</b> 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.<br><br>Clear candidates for inclusion:<br>    - "<b>Use ctrl/alt for straight line drawing</b>"  - Sounds like a good idea. I'll test it a bit and to check how well it works<br></div><div>    - "<b>Invert F/Ctrl-F</b>"  -  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.<br>   <br><br></div><div>Candidates which might need a little closer review first:<br></div><div>   - <b>Joining strokes together</b>  - 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<br></div><div><br></div><div>   - <b>"Ctrl-H  for Hiding Points"</b> - My big question would have to be - how well is this working for you guys (Daniel/Matias?). <br></div><div>      (On a side note, it's better to just do   "<span style="font-family:monospace,monospace">ToolSettings *ts = CTX_data_tool_settings(C)</span>;"  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)<br><br></div><div><br></div><div><b>3) Grease Pencil and Compositor</b><br></div><div>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.<br><br></div><div>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). <br><br><br></div><div><b>4) Brush Presets</b><br></div><div>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)?<br><br><br></div><div><b>5) Object/Bone Parenting</b><br></div><div>First, it's great that we've finally got this in place :D<br><br></div><div>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?)     <br><br>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)<br></div><div><br></div><div><br></div></div>
</div><br></div></div></div></div></div>