Sync timer interrupt with external interrupt trigger

I'm having a little trouble with this snippet of code, and am a bit stumped at the moment.

I have an external interrupt being triggered on pin 2 at 120Hz, and I'm trying to sync that signal with a timer that is setup to output at the same frequency with a specified duty cycle (for an AC dimmer, using triac)

Anyone notice if I'm doing this wrong? I don't think my math is correct. Oscilloscope shows what appears to be the correct signal, but I don't think it is syncing properly because the light output rolls.

#define outPin 2
#define INTERRUPT_NUM 0		// Use interrupt 0 on Pin 2 (yellow wire)

double dimmerLevel = 0.5;
unsigned long timeOn;
volatile unsigned long timer1_counter;
volatile bool syncPulse = false;				// Interrupt signal

void setup()
{
	pinMode(outPin, OUTPUT);
	digitalWrite(outPin, LOW);

	Serial.begin(115200UL);

	// calculate duty cycle time
	// 100% duty cycle = 8.333ms or 8333.33us
	// 50% duty cycle = 8333.33 * 0.5 = 4166.665us
	// 1 interrupt = 32.64us (8333.33us - 8.333ms / 255 steps)
	// therefore 50% duty cycle = 4166.665 / 32.64 = 127.655 interrupts fired
	// 1 full cycle = 255 timer1_counter ticks
	timeOn = (unsigned long)(floor((8333.33 * dimmerLevel) / 32.64));

	cli();	// diable global interrupts
	TCCR1A = 0;
	TCCR1B = 0;

	timer1_counter = 0;	// preload timer 65536-16Mhz/256/2Hz

	// 32.64us = (1000ms / 120Hz = 8.333ms or 8333.33us) / 255 steps (60Hz mains calculate minimum time slice length)
	// 1 / 16Mhz = 0.0625us (clock speed)
	// 522.24 = (32.64us / .0625us)
	OCR1A = 522;	// set compare match register to our desired timer count

	TCCR1B |= (1 << WGM12);	// turn on CTC mode
	TCCR1B |= (1 << CS10);	// no prescaler
	TIMSK1 |= (1 << OCIE1A);	// enable timer compare interrupt
	
	sei();	// enable global interrupts

	attachInterrupt(INTERRUPT_NUM, zero_cross_int, RISING);	// Choose the zero cross interrupt # from the table above
}

void zero_cross_int() {
	// Firing angle calculation: 60Hz = 8.333ms (1000ms / (60Hz * 2))
	// 1ms = 1000us
	// (8333.333 - 8.333ms ) / 128 steps = 65.1us
	// (8333.333 - 8.333ms ) / 255 steps = 32.64us

	// Here we are going to measure the frequency of the zero-cross interrupts by keeping track of
	// the last 2 interrupts and measuring the time between then. We could optionally average the last 5 for more less jitter.

	syncPulse = true;
	TCNT1 = 0;	// reset the timer1 count
	timer1_counter = 0;
}

ISR(TIMER1_COMPA_vect)	// ISR routine
{
	if(timer1_counter == 0){
		// on
		digitalWrite(outPin, HIGH);
	}

	if(timer1_counter == timeOn){
		// off
		digitalWrite(outPin, LOW);
	}

	timer1_counter++;
	if(timer1_counter >= 255)
		timer1_counter = 0;
}

void loop()
{
	// do nothing
}

Am I possibly not resetting the timer interrupt correctly?

Oscilloscope shows what appears to be the correct signal, but I don't think it is syncing properly because the light output rolls.

Shows where? What pin you are using to control lights? D2 has to be input, basically , you need software PLL, where you synchronize input to output with specific "pre-scaler" or multiplication

Sorry there is a typo in my code sample - very first line doh.. outPin should be 3, not 2 (interrupt 0).

The oscilloscope shows on pin 3 (output) a square wave with a duty cycle at the amount expected according to what I coded, but it starts a little early which results in a 'rolling' dim level on the light bulb. It looks to me that the square wave is happening before the peak of the zero cross input - which I don't quite understand.

I guess I'm not understanding - I am after software PLL but I'm not quite sure what I'm doing wrong. I thought resetting the timer counter would basically sync my timing with the external interrupt (zero crossing), but apparently not. I've calculated all the timings for the timer (no prescaler) and it should fire precisely every 32.64 microseconds, giving me 255 steps of brightness resolution on my output.

Probably, you don't need PLL after all, except high resistance to interference required.
What may be not correct, is CTC mode , timer1 is reseting twice in this case, on external interrupt and internally on TOP, try normal mode not setting WGM12 bit in this line:

TCCR1B |= (1 << WGM12); // turn on CTC mode

I tried that, the timer does not fire at all in that case.

The reason I'm using CTC mode is that I'm running the timer at clock speed (no prescaler) with a CTC count of 522 (instead of overflowing at 65535). This way, my timer fires every 32 microseconds. So for every zero cross pulse, my timer should trigger 255 times in between a single cycle (8333.33 microseconds).

I may need to do some more digging to determine the proper way to sync the timer pulses, since I want the timer to start its on/off PWM cycle when the zero cross happens. Or maybe my math is wrong.

I should say this in the beginning, you are using quite expensive in performance perspective, way to implement simple delay-timer. Why would I fire 255 interrupts if I need only one? Why not use normal mode with OCIE1A enable, loading OCR1A with value, proportional to 522 x n, where n is 0- 255 integer? Re-load 'd be inside external interrupt subroutine, if brightness has to be changed.
I have a project, where I really need 128 interrupts during 1 cycle of mains, as arduino doesn't have DMA feature and sampling ADC to memory has to be done "manually".
Here is the code (you may download full sketch from the link below - power quality analyzer):

ISR(TIMER1_COMPB_vect)
{ 
 if ( smpl_Nmbr < FFT_SIZE )
 {
   x_r[smpl_Nmbr]  = ADC - adc_Offst; // 
 } 
 smpl_Nmbr++;                         // Last One N = 128
}

ISR(INT0_vect) {                      // PLL - Primary Input         
   static int8_t delt_Bins = 0;

   delt_Bins++;
   if ( delt_Bins >= NBINS )
   {
     delt_Bins = 0;

     int16_t cntr_Tmr1 = TCNT1;
  
   cntr_TmrF = cntr_Tmr1 + (long) smpl_Time * smpl_Nmbr;   // Period Clock
   
  if ( smpl_Nmbr > FFT_SIZE ) smpl_Time++;   
  if ( smpl_Nmbr < FFT_SIZE ) smpl_Time--;       
    
   OCR1A  = smpl_Time;
   OCR1B  = smpl_Time;
   TCNT1  = 0;
   
   smpl_Nmbr = 0;
   smpl_Flag = 1;                     // New Data FLAG
   }
}

As you can see, I use PLL, adjusting timer speed (in your case 522 constant) inside INT0. Anyway, it necessary in my project, but doesn't make sense in yours, simply load OCR1A ones, and inside COMPA_vect you switch pin ON , triack doesn't need to be switch off...

Magician - thanks man great comments.. I figured a constant timer source would make more sense since I do need 255 levels of resolution, but it would make more sense to use a lower resolution timer interrupt based on the dim value. I realized my code was a little incorrect in terms of the timing on the triac control - I need to delay a period of time before turning the triac on briefly, not the other way around, so that it ends up being turned on in the middle of the phase instead of at the start of the phase for a period of time.

I'm now also avoiding use of digitalWrite after seeing how much it degrades performance in a timer that runs every 8us. Still experimenting, will try implementing your PLL example once I fully understand it.