Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

One light position per camera position #40

Closed
p-mc-grath opened this issue Jul 1, 2020 · 9 comments
Closed

One light position per camera position #40

p-mc-grath opened this issue Jul 1, 2020 · 9 comments
Labels
question Question, not yet a bug ;)

Comments

@p-mc-grath
Copy link

Hi, I created a file with a bunch of camera coordinates and a file of light coordinates. When I generated the pictures, I noticed that there is one light per coordinate in every scene. What I actually want is one light per coordinate per scene, i.e. there can only be one light source per image and I want to be able to specify its position with coordinates. Is that possible somehow?

@themasterlink
Copy link
Contributor

Hey,

I am sure it is possible, but first let me understand what you are trying to do here.

What you want to do is that for each camera frame there is only one light? Do I understand that correctly?

@themasterlink
Copy link
Contributor

Assuming this was your question, the current implementation does not offer that feature. However, it is super easy to implement.

I am going to highlight the necessary steps and if you need further assistance just ask.

The LightLoader is similar to the CameraLoader, which we draw inspiration of for this change you want:

This function is called for each line specified in your object file, right now. It creates a new light object for each line of the config.

def _add_light_source(self, config):
""" Adds a new light source according to the given configuration.
:param config: A configuration object which contains all parameters relevant for the new light source.
"""
# Create light data, link it to the new object
light_data = bpy.data.lights.new(name="light", type="POINT")
light_obj = bpy.data.objects.new(name="light", object_data=light_data)
bpy.context.collection.objects.link(light_obj)
light_data.type = config.get_string("type", 'POINT')
light_obj.location = config.get_list("location", [0, 0, 0])
light_obj.rotation_euler = config.get_list("rotation", [0, 0, 0])
light_data.energy = config.get_float("energy", 10.)
light_data.color = config.get_list("color", [1, 1, 1])[:3]
light_data.distance = config.get_float("distance", 0)

What you could do is:

  1. Move this creation step to the __init__() the creation happens in L44-L47.

  2. Save the created light light_obj in a member -> self.light_obj.

  3. Use in the fct. _add_light_source the self.light_obj instead of the light_obj

  4. Each call now sets the location value, to track this change you need to save this value in a keyframe (check the blender docu for more info on keyframes).
    This is done as in the CameraLoader:

    # Store new cam pose as next frame
    frame_id = bpy.context.scene.frame_end
    self._insert_key_frames(cam, cam_ob, frame_id)
    bpy.context.scene.frame_end = frame_id + 1

  5. However, this increase the frame end counter of blender, which we don't want to do. I suggest adding a member to the __init__ light counter -> self.light_counter, which is counted up everytime _add_light_source is called.

  6. This counter can then be used like it is done inside of the _insert_key_frames call:

self.light_obj.keyframe_insert(data_path='location, frame=self._light_counter) # the light counter needs to be inited with 0
self.light_obj.keyframe_insert(data_path='rotation_euler', frame=self._light_counter)
self._light_counter += 1

I hope this helps if you have any further questions, ask away.

Thanks for the idea, we will add a future like this in a future version of BlenderProc.

@themasterlink themasterlink added the question Question, not yet a bug ;) label Jul 1, 2020
@p-mc-grath
Copy link
Author

Thanks so much for the quick and really helpful response!! I will tackle this in the upcoming week. If it is ok with you, I will close the issue once I am done and hence sure that I do not have any more questions.

@p-mc-grath
Copy link
Author

p-mc-grath commented Jul 2, 2020

So basically the only script we are changing would be LightModule.py and if I only care about changing the location, it would look like this?

    def __init__(self, config):
        Module.__init__(self, config)

        self._light_counter = 0

        # Create light data, link it to the new object
        self.light_data = bpy.data.lights.new(name="light", type="POINT")
        self.light_obj = bpy.data.objects.new(name="light", object_data=self.light_data)
        bpy.context.collection.objects.link(self.light_obj)

        self.cross_source_settings = self.config.get_raw_dict("cross_source_settings", {})
        self.light_source_collection = ItemCollection(self._add_light_source, self.cross_source_settings)

    def _add_light_source(self, config):

        self.light_obj.location = config.get_list("location", [0, 0, 0])
        self.light_obj.keyframe_insert(data_path='location', frame=self._light_counter) 
        self._light_counter += 1

and my config would need to contain something like:

{
      "module": "lighting.LightLoader",
      "config": {
          "lights": [
              {
                "type": "POINT",
                "energy": 1000
              },
         ],
         "path": "<args:0>",
         "file_format": "location"
      }
}

Logically this should work but my RGB images are all very dark. I think there is a light source somewhere, as the model is lit slightly from one side but really subtly. I had a look at the corresponding Camera Scripts:

  • do I need to set the initial light location somehow?
  • do I need to transform the coordinates (the input to args:0 is exactly the same as for the camera coordinates apart for the missing rotation values)

@themasterlink
Copy link
Contributor

Yes I think you are missing setting the energy value.

light_data.energy = config.get_float("energy", 10.) 

I would add this to the top:

def __init__(self, config):
    Module.__init__(self, config)

    self._light_counter = 0

    # Create light data, link it to the new object
    self.light_data = bpy.data.lights.new(name="light", type="POINT")
    self.light_data.energy = self.config.get_float("energy", 10.)  # <- this line is new
    self.light_obj = bpy.data.objects.new(name="light", object_data=self.light_data)
    bpy.context.collection.objects.link(self.light_obj)

    self.cross_source_settings = self.config.get_raw_dict("cross_source_settings", {})
    self.light_source_collection = ItemCollection(self._add_light_source, self.cross_source_settings)

Then they should have the correct lighting.

I think the rest should be already correct, check it out and tell me if it worked.

@themasterlink
Copy link
Contributor

Oh one addition, you have to move the energy level on the samel level as the "path".

{
      "module": "lighting.LightLoader",
      "config": {
         "energy": 1000,
         "type": "POINT",
         "path": "<args:0>",
         "file_format": "location"
      }
}

@p-mc-grath
Copy link
Author

It worked like a charm!! Thank you so much. I really appreciate your efforts, thanks again :)

@themasterlink
Copy link
Contributor

@pmcgrath249 please cite our work if you publish ;)

@p-mc-grath
Copy link
Author

Of course! Keep up the great work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Question, not yet a bug ;)
Projects
None yet
Development

No branches or pull requests

2 participants