[Blender 2.8] Fix texture paint image nodes not connecting to any node socket (when added via missing data panel).
ClosedPublic

Authored by Sebastian Parborg (zeddb) on Sep 6 2018, 8:28 PM.

Diff Detail

Repository
rB Blender
Sebastian Parborg (zeddb) edited the summary of this revision. (Show Details)

I've updated this to fix the issue described in this ticket too: https://developer.blender.org/T56525

I hope that is not a problem. Otherwise I could just create a separate pull request for that one.

Brecht Van Lommel (brecht) requested changes to this revision.Sep 10 2018, 4:25 PM

Looks quite good, some comments.

I also noticed there's a few more improvements we can do in this area if you're interested:

  • Keep the button to add material slots available also when some slots were already added. Right now it seems to disappear, which is a bit odd since you might want to have multiple image textures.
  • I suggest to join the "Missing Data" and "Slots" panel into one, so that it's just "Slots". Then there is one place to add everything since it's a bit odd how these jump around.
  • The nodes could be positions a bit better in the node editor, to the left of the socket they are connected to instead of overlapping. Perhaps a general utility function could be added for this.
source/blender/blenkernel/intern/material.c
1033

Code style: { on new line for functions.

1033

Use int instead of short, it tends to be faster on modern CPUs and there's no reason to save space here.

1042

The code is fine as is I think, no need to use a pointer.

1049

Code style: { on new line for functions.

source/blender/editors/sculpt_paint/paint_image_proj.c
5617–5624

To get rid of the hardcoded numbers, you could use here:

enum {
    LAYER_BASE_COLOR,
    LAYER_SPECULAR,
    ....
};

static const EnumPropertyItem layer_type_items[] = {
    {LAYER_BASE_COLOR, "BASE_COLOR", 0, "Base Color", ""},
    {LAYER_SPECULAR, "SPECULAR", 0, "Specular", ""},
    ....
}
5696

Code style: space between ) {

5704

Code style: use /* */ for comments.

5707–5710

Best to just remove this from the enum and remove this code until it's actually there. Painting emission maps is not super common so not so important to have yet.

5740

This can return NULL (in rare cases), so needs to be checked.

5744

Emission is not non-color data, but indeed all the others are.

This revision now requires changes to proceed.Sep 10 2018, 4:25 PM
Sebastian Parborg (zeddb) marked 10 inline comments as done.

I've updated the code based on your feedback

This revision is now accepted and ready to land.Sep 10 2018, 6:56 PM
This revision was automatically updated to reflect the committed changes.